How to choose the correct memory allocator?

First of all, I apologise for opening a new thread related to memory issues - I know there have been quite a few already, but this question (at least in my case) still remains unanswered. For the record, I have read through #955 and a bunch of issues related to that multiple times. I still need some help here, and I didn't know which of the existing issues to revive, so here's a new one.
The setup
I have created four test sharp operations (https://github.com/juona/sharp-memory-experiment/blob/master/generatePrintFile.js):

Composite - takes 3 images, performs resize, extract and extend on each one of them, converts two of them into buffers using toBuffer() and performs the composite operation to produce the final result in the form of a PNG file. This is the original function that I want to work well in my application.
Single - takes 1 image, performs resize, extract and extend and writes the result into a PNG file without using toBuffer().
Simple to file - takes a single image and writes it to a new file without any sharp operations.
Simple to buffer - takes a single image and calls toBuffer() on it and does nothing else, i.e. it just does nothing with the result.

The tests
Now, each of these operations would be run many times in a row under four different conditions to see what influence caching and concurrency have on the memory consumption:

Sharp concurrency on (sharp.concurrency(0)) and sharp cache on (sharp.cache(true));
Sharp concurrency on (sharp.concurrency(0)) and sharp cache off (sharp.cache(false));
Sharp concurrency off (sharp.concurrency(1)) and sharp cache on (sharp.cache(true));
Sharp concurrency off (sharp.concurrency(1)) and sharp cache off (sharp.cache(false));

Also, the previously described sharp operations would be run in chunks of 10, e.g. I would simultaneously launch 10 "composite" operations, let the program wait until they all finish, record the RSS and do that again.
This would be repeated until either the loop is repeated 100 times (so I have 100 readings of RSS) or the memory reaches 2GB on my Manjaro/1GB on my Mac.
Finally, the images used were two rather large (3400x2000, 11MB) and one small PNG files, all available in the repository (link below).
The whole test suite with instructions is available here: https://github.com/juona/sharp-memory-experiment.
The important bit
I ran these tests using three different memory allocators:

On a Manjaro system using the default Manjaro allocator. I am not very good at computers/linux, so I don't know which one it is. :]]
On the same Manjaro system using jemalloc, as suggested in other threads, namely #955.
On a Mac (10.14.6) using its default allocator.

The results
Here are the graphs of how memory consumption changes with each iteration. The X axis represents the iteration number (so, e.g. iteration 15 means the 15th time that a chunk of 10 sharp operations was finished). The Y axis is RSS as printed by NodeJS itself in MB.

As you can see, the results using the default Manjaro allocator are so bad, that we can just ignore them.
On a Mac, the operations which do not involve buffers ("single" and "simple to file") seem to only consume a little bit of memory, and the consumption is basically constant. However, the RSS changes during the "simple to buffer" test look quite odd (don't know how to explain them), and during the "composite" operation (optimising which was my initial goal) the RSS is constantly increasing.
On the Manjaro system with jemalloc enabled, the behaviour is almost identical, except that the memory usage during "simple to buffer" is significantly higher and takes only 13 rounds to reach the 2GB mark.
Questions

Would anyone who claims that they do not experience memory issues when running sharp on their system, care to try my tests out and see what happens?
Given that jemalloc does not help me, how would I know which allocator to choose to fix or at least mitigate the problem? Perhaps there is something wrong with my code?
How could these results be explained? E.g. when using the default Manjaro allocator, there appears to be some sort of RSS saturation. I know others have seen it too but I still don't understand why it just does not keep increasing.
More of a note rather than a question, but IMHO the "use a different memory allocator" suggestion is rather weird. I am yet to encounter software which would officially instruct users that "if you have problems with high RSS, use a different memory allocator". And in case libvips is optimised to a specific allocator, then I'd expect the answer to question 2 to be a really easy one, right? If it's not, then isn't that an indicator of some peculiarities inside libvips?

Here's a graph that better shows how Mac deals with simpleToBuffer:

And here's another one showing that on Mac RSS does only ever increase during the composite operations:

Hello @juona,
I tried your test in pyvips, the libvips Python binding:
import sys
import pyvips

for i in range(int(sys.argv[1])):
    print(f"loop {i} ...")

    a = pyvips.Image.new_from_file(sys.argv[2], access="sequential")
    b = pyvips.Image.new_from_file(sys.argv[3], access="sequential")
    c = pyvips.Image.new_from_file(sys.argv[4], access="sequential")

    a = a.crop(10, 10, 1000, 1000).resize(0.9)
    b = b.crop(10, 10, 1000, 1000).resize(0.9)
    c = c.crop(10, 10, 1000, 1000).resize(0.9)

    background = pyvips.Image.black(2000, 2000)
    out = background.composite([c, b, a], "over",
                               x=[900, 450, 0], y=[0, 450, 900])

    out.write_to_file(sys.argv[5])

Then ran with:
./composite3.py 10000 ~/pics/k2.jpg ~/pics/k4.jpg ~/pics/wtc.jpg x.png

And watched RES in top:

loop
RES (mb)

10
105

30
128

50
131

100
142

200
155

300
145

400
139

500
141

I don't think there's a leak in libvips itself
I would expect it to take at least 200 iterations for memory use to stabilize due to heap fragmentation

Memory management in node, especially when there are external libraries involved, is complex, with many interacting systems, most of which know little about each other :(
In your buffer case, could it be that the node GC is simply not firing? Have you tried forcing a GC?
Start node with:
$ node --expose-gc file.js

Then add this to your loop:
global.gc();

No use in production, of course, but it might help explain the memory behaviour you are seeing.

Thanks @juona for these detailed test cases.
jemalloc helps reduce fragmentation in long-running, multi-threaded processes, at the cost of slightly higher peak memory usage (RSS), as it allocates from multiple arenas.
The logic in the composite test function appears to keep all intermediate images in scope, so they won't become eligible for garbage collection until after the whole test function is complete, which could explain the pattern you're seeing.
As John suggests, forcing V8 to run garbage collection can help better determine the real peak memory requirement for this task as it may reduce the number of new allocations. Remember it's still up to the OS to request that a process returns unused memory, so RSS can still include this freed memory when it is fragmented or smaller than the current page size.

I've check on our production infrastructure while we do some heavy processing with Sharp (We resize pictures and generate tiles for them).
Here is an example of our graphana charts, it seems that it causes one of our service to crash (After ~3k calls to sharp.tiles() ) (Strangely, I wasn't able to reproduce it locally).

Just reading into this as we are seeing similar activity.

This is an image processing / resizing service that we are currently benchmarking for production, the only code used is:
return sharp(binary).rotate().resize({ width: size, height: size }).png().toBuffer().then(thumb => {
  return ThumbnailsBucket
    .file(`${name}@${size}x${size}`, { metadata: { width: size, height: size } })
    .save(thumb)
})

Merci beaucoup @lovell.
It fixes the ram consumption with jemalloc.so.1 (~v3.6) on debian Stretch, jemalloc.so.2 (~v5) on debian Buster and musl on alpine.
But it seems to have an intensive usage of the hdd despite not using it at all (I can't explain it but it's not related to Sharp)

So sorry for a late reply, I was busy trying to get the feature that I needed working on time.
So to do that I rewrote the whole thing using pyvips last week and indeed there are no memory problems even with the default allocator, so I am very happy that there is a way around this. As I have to use Python anyway, it does not make a huge difference to me, but it would be nice to somehow find a way to make sharp work eually well for everybody, as the lib is really useful.
 Please don't close this yet!
Thanks.

@juona Were you able to make any progress with this?

Hello,
Sadly, no. I am sorry for:

A late reply.
Not going through with this.

My attention is 100% on the project, so I don't really have time left for this. Feel free to close this, and if I free myself up enough, I'll do the tests in the future.

Thank you for the update, I'll close this for now but please feel free to reopen with more details if further help is required.


HDDS-5033. SCM may not be able to know full port list of Datanode after Datanode is started.

What changes were proposed in this pull request?
When SCM HA is enabled, after restart DN, the SCM may not know the full ports of that DN.
The issue is:
SCMNodeManager just record the DatanodeDetails once during register. But for DN, it won’t record the admin, server, client port into DatanodeDetails until its ratis server is up. Thus there is contention here: if the register request is reported before ratis server is up, SCM won’t know full port list of that DN.
The solution is
If SCM HA is enabled, OzoneContainer#start() will be called multi-times from VersionEndpointTask. The first call should do the initializing job, the successive calls should wait until OzoneContainer is initialized.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5033
How was this patch tested?
CI and integration test inside tencent.

@bshashikant @nandakumar131 Please take a look at this bug fix of SCM HA. Thanks

@GlenGeng , can you explain why OzoneContainer#start() needs to be called multiple times if SCM HA is enabled?

@GlenGeng , can you explain why OzoneContainer#start() needs to be called multiple times if SCM HA is enabled for registering to the same SCM?

Please check
public class SCMConnectionManager
    implements Closeable, SCMConnectionManagerMXBean {
  private static final Logger LOG =
      LoggerFactory.getLogger(SCMConnectionManager.class);

  private final ReadWriteLock mapLock;
  private final Map<InetSocketAddress, EndpointStateMachine> scmMachines;

If SCM HA is enabled, there will be 3 VersionEndpointTask created, one for each SCM.

@GlenGeng , can you explain why OzoneContainer#start() needs to be called multiple times if SCM HA is enabled for registering to the same SCM?

Actually no, each call is for different SCM.


Sidebar missing until mermaidjs finishes loading

Hi!
Screenshot 1:
Sidebar missing, mermaidjs loading

Screenshot 2:
Sidebar appeared after mermaidjs loaded

This is a problem for me, because mermaidjs takes ~ 1 minute to load.
Problem appeared after website update recently.
Browser:
Google Chrome
Version 126.0.6478.126 (Official Build) (64-bit)

This was done as part of https://github.com/erlang/otp/pull/8197. Maybe @Benjamin-Philip has some idea about how it can be improved?

Hi @vlad-ngn , can you replicate the issue with Elixir's GenServer docs?
The mermaid setup in Erlang was more or else copied as is from Elixir's. It's very likely that this issue affects Elixir's docs too.

Hi @vlad-ngn , can you replicate the issue with Elixir's GenServer docs?
The mermaid setup in Erlang was more or else copied as is from Elixir's. It's very likely that this issue affects Elixir's docs too.

Yes, same issue with Elixir's docs

Screenshots

In that case, you should file an issue upstream in ExDoc or Elixir repo. Please tag me as well.

Thanks!
Created issue in the ExDoc repo: https://github.com/elixir-lang/ex_doc/issues/1941


IconHelper leaves unsed API allocated memory

Your working with the Windows API is dirty. You don't frees API allocated memory. In description of ExtractIconEx states under "Remarks" you have to destroy the icons, that means calling DestroyIcon for each icon handle (your IntPtr's).
I'm not familiar right now with GIT so I don't fix it in code, but introduces a "TODO" comment with links to the MSDN descriptions of both functions.

Thanks for your PR. I will check this out.

Maybe I will remove this completely:
It has no real additional benefit, is not compatible with Mono and the extracted icons do not look good. I prefer using two static icons: the KeePass-Icon for databases and a generic document icon for all other files.


${} instead of @{} as bounding syntax.

Fixes #
Description

Specific Changes

Testing

Pull Request Test Coverage Report for Build 106367

7 of 7 (100.0%)  changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 84.255%

Totals

Change from base Build 106328:
0.0%

Covered Lines:
8733

Relevant Lines:
10025

💛  - Coveralls


function that drops variables from meta and data

#642

I am closing this in favor of:
https://github.com/Quantipy/quantipy/pull/632


Add dynamic fields query

Added dynamic_field, dynamic_object_field and dynamic_fields queries to client. They all return a type that includes the serde_json::Value and Object. Also added a dynamic_fields_on_wrapped_objects function that uses the owner to query for dynamic fields.
My understanding from the GraphQL code is that MoveObject's bcs is the more generic Object, which should work just fine.
Finally, the last few changes (3be26a8b and 43fbd400) were informed by integrating this sui-sdk-types and sui-graphql-client as dependencies in another project. The CLI there relies heavily on dynamic fields queries, on wrapped objects.

The main dynamic field changes LGTM, but there are some changes related to fetching objects and their contents that I think may need some work along the lines of what we talked about a week or so ago:

That we would ideally support three or four representations of objects: JSON, BCS (raw bytes), Serializable types, and a kind of dynamic MoveValue (similar to JSON, but native to Move, similar to annotated::MoveValue in the Move world).
This implies we would need some way to specify which kind of representation we want when querying for things that return move values (dynamic fields included).

I wonder if there's a way we can address that separately though (in another PR) because it seems a little out of scope of this PR, even though it has implications for what's going on here.

One approach to this would be to tweak the API to instead of having a single function that takes all the parameters, instead you have a request builder paradigm (like reqwest) where you build up your request and specify the "format" you want. The question here though, is, do you ever want multiple types of formats at the same type. Also how much do we want to bake into this "builtin" client vs pushing folks to craft their own queries

Builder API is intriguing -- I think it's fairly unlikely that someone would want multiple representations of a type (unlikely enough that we don't need to support it in the built-in client), but I think the SDK does need to have some native notion of a "Move Value", because in the GraphQL schema these are represented as scalars, which means it's not really a question of writing a custom query -- these types exist in the leaves of the query, so even if someone wrote a custom query, they would need some help to translate the JSON blob at that leaf into a Rust representation of a Move value and that is a universal problem.


Could not detect abi for version 1.6.4 and runtime electron

[x] I have read the contribution documentation for this project.
[x] I agree to follow the code of conduct that this project follows, as appropriate.
[x] I have searched the issue tracker for an issue that matches the one I want to file, without success.

Please describe your issue:
❯ electron-forge -V
✔ Checking your system
2.8.0

❯ cat package.json | json scripts.start
electron-forge start

❯ npm start

><EMAIL_ADDRESS>start /Users/zeke/ummoji/ummoji-desktop
> electron-forge start

✔ Checking your system
✔ Locating Application
✖ Preparing native dependencies

An unhandled error has occurred inside Forge:
Could not detect abi for version 1.6.4 and runtime electron
Error: Could not detect abi for version 1.6.4 and runtime electron
    at Object.getAbi (/usr/local/lib/node_modules/electron-forge/node_modules/node-abi/index.js:42:9)
    at _callee6$ (/usr/local/lib/node_modules/electron-forge/node_modules/electron-rebuild/lib/rebuild.js:152:37)
    at tryCatch (/usr/local/lib/node_modules/electron-forge/node_modules/electron-rebuild/node_modules/regenerator-runtime/runtime.js:63:40)
    at GeneratorFunctionPrototype.invoke [as _invoke] (/usr/local/lib/node_modules/electron-forge/node_modules/electron-rebuild/node_modules/regenerator-runtime/runtime.js:337:22)
    at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/usr/local/lib/node_modules/electron-forge/node_modules/electron-rebuild/node_modules/regenerator-runtime/runtime.js:96:21)
    at GeneratorFunctionPrototype.tryCatcher (/usr/local/lib/node_modules/electron-forge/node_modules/bluebird/js/release/util.js:16:23)
    at PromiseSpawn._promiseFulfilled (/usr/local/lib/node_modules/electron-forge/node_modules/bluebird/js/release/generators.js:97:49)
    at /usr/local/lib/node_modules/electron-forge/node_modules/bluebird/js/release/generators.js:201:15
    at _rebuild (/usr/local/lib/node_modules/electron-forge/node_modules/electron-rebuild/lib/rebuild.js:613:18)
    at rebuild (/usr/local/lib/node_modules/electron-forge/node_modules/electron-rebuild/lib/rebuild.js:624:28)
    at _callee$ (/usr/local/lib/node_modules/electron-forge/dist/util/rebuild.js:37:67)
    at tryCatch (/usr/local/lib/node_modules/electron-forge/node_modules/regenerator-runtime/runtime.js:64:40)
    at GeneratorFunctionPrototype.invoke [as _invoke] (/usr/local/lib/node_modules/electron-forge/node_modules/regenerator-runtime/runtime.js:355:22)
    at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/usr/local/lib/node_modules/electron-forge/node_modules/regenerator-runtime/runtime.js:116:21)
    at GeneratorFunctionPrototype.tryCatcher (/usr/local/lib/node_modules/electron-forge/node_modules/bluebird/js/release/util.js:16:23)
    at PromiseSpawn._promiseFulfilled (/usr/local/lib/node_modules/electron-forge/node_modules/bluebird/js/release/generators.js:97:49)

Console output when you run electron-forge with the environment variable DEBUG=electron-forge:*. (Instructions on how to do so here). Please include the stack trace if one exists.
❯ DEBUG=electron-forge:* npm start

><EMAIL_ADDRESS>start /Users/zeke/ummoji/ummoji-desktop
> electron-forge start

WARNING: DEBUG environment variable detected.  Progress indicators will be sent over electron-forge:lifecycle
  electron-forge:lifecycle Process Started: Checking your system +0ms
  electron-forge:lifecycle Process Succeeded: Checking your system +26ms
  electron-forge:runtime-config setting key: verbose to value: false +8ms
WARNING: DEBUG environment variable detected.  Progress indicators will be sent over electron-forge:lifecycle
  electron-forge:lifecycle Process Started: Locating Application +0ms
  electron-forge:project-resolver searching for project in: /Users/zeke/ummoji/ummoji-desktop +1ms
  electron-forge:project-resolver electron-forge compatible package.json found in /Users/zeke/ummoji/ummoji-desktop/package.json +6ms
  electron-forge:lifecycle Process Succeeded: Locating Application +1ms
  electron-forge:lifecycle Process Started: Preparing native dependencies +1ms
  electron-forge:lifecycle Process Failed: Preparing native dependencies +4ms

An unhandled error has occurred inside Forge:
Could not detect abi for version 1.6.4 and runtime electron
Error: Could not detect abi for version 1.6.4 and runtime electron
...

What command line arguments are you passing?
(none)
What does your config.forge data in package.json look like?
It's the default from running electron-forge import.
  "config": {
    "forge": {
      "make_targets": {
        "win32": [
          "squirrel"
        ],
        "darwin": [
          "zip"
        ],
        "linux": [
          "deb",
          "rpm"
        ]
      },
      "electronPackagerConfig": {},
      "electronWinstallerConfig": {
        "name": ""
      },
      "electronInstallerDebian": {},
      "electronInstallerRedhat": {},
      "github_repository": {
        "owner": ""
      },
      "windowsStoreConfig": {
        "packageName": ""
      }
    }
  }

Please provide either a failing minimal testcase (with a link to the code) or detailed steps to
reproduce your problem. Using electron-forge init is a good starting point, if that is not the
source of your problem.
Code is at https://github.com/ummoji/ummoji-desktop/pull/9

@zeke node-abi definitely supports this version of Electron.  https://github.com/lgeiger/node-abi/blob/master/index.js#L6
Can you try a quick reinstall
npm uninstall -g electron-forge
npm install -g electron-forge

It looks like NPM didn't update the transitive dependency when it updated electron-forge, that and forge is up to 2.8.3 now so always good to be up to date 😄

Bumping to 2.8.3 did the trick. Thanks.


Feats/bucket lord

closes #12

it's implemented, it runs, and it seems to be about as efficient as the previous solution—but I don't know if it's an actual improvement in terms of padding efficiency.
The buckets are likely very sparsely populated (with n_buckets² buckets total, that's a lot), so maybe we want fewer buckets and binning in comparable groups, rather than exact length matches (maybe we want n_buckets ** ½ or something).
I'll see if I can come up with stats in terms of how much padding we observe with this solution.

The buckets are likely very sparsely populated (with n_buckets² buckets total, that's a lot), so maybe we want fewer buckets and binning in comparable groups, rather than exact length matches (maybe we want n_buckets ** ½ or something).

Indeed. I think that the original 1-d buckets were probably already unnecessarily sparse due to being based on exact rather than quantized lengths. There is a tradeoff between minimizing the amount of padding and minimizing the amount of correlation between examples in a batch. Being extremely aggressive with minimizing padding may not be optimal for training, if it results in a very unshuffled data order (and large memory use).
I think that a variable-granularity binning with finegrained bins near the mode of the lengths and rough outside would be good:

the amount of extra padding is minimized for the very common batches near the mode of the distribution. These have a lot of data to choose from, so waiting for the batch to fill up will not lead to it always containing the exact same sentences.
the outliers are rare enough to not matter, so no sense binning them very exactly.

The buckets are likely very sparsely populated (with n_buckets² buckets total, that's a lot), so maybe we want fewer buckets and binning in comparable groups, rather than exact length matches (maybe we want n_buckets ** ½ or something).

Indeed. I think that the original 1-d buckets were probably already unnecessarily sparse due to being based on exact rather than quantized lengths. There is a tradeoff between minimizing the amount of padding and minimizing the amount of correlation between examples in a batch. Being extremely aggressive with minimizing padding may not be optimal for training, if it results in a very unshuffled data order (and large memory use).

Memory use isn't a concern (there's still only pool_size items considered at any point). I think the system at least gives reasonable guarantees for shuffling near the mode of the length distribution. So the problem is likely more specifically the tail of the distribution.

I think that a variable-granularity binning with finegrained bins near the mode of the lengths and rough outside would be good:
1. the amount of extra padding is minimized for the very common batches near the mode of the distribution. These have a lot of data to choose from, so waiting for the batch to fill up will not lead to it always containing the exact same sentences.

2. the outliers are rare enough to not matter, so no sense binning them very exactly.

That's not easy to pre-compute, and requires different parameters per language or per task. It's not impossible, but it's going to be very involved. My concrete approach to that would be to try to automate that in the config config while reading the corpora length, e.g. with a process similar to the following:

fit some random variable to the observed length distribution on a subset of the data
compute equiprobable bins through the cdf
use bins from 2. to define lists of bucket sizes,
store in a new task key

We could obviously skip 1 and 2 and rely on hard observed data if we compute the full stats of each corpus. Do remark that in any case, this does involve configuring bucket sizes in the task.
If on the other we only care about ensuring that tail items are lumped in a single bucket, then I would consider just defining an outlier bucket by cutting off anything below an expected max length (say 128 or less), which can already be done by setting a lower n_bucket value.
In the present 2D case, we could also rely on length ratios to define how many of the offset diagonals we tolerate before sending an example to the outlier bucket.

There is a tradeoff between minimizing the amount of padding and minimizing the amount of correlation between examples in a batch.

If we're stressed out about wrapping around the full corpus and constructing batches with very similar examples (which is justifiable for LR langs in my opinion), then we can

remove the infinite cycling from the dataset,
use a flag mark when the example stream has been exhausted
restart the iteration over the example stream when we've hit self.is_empty()

Memory use isn't a concern (there's still only pool_size items considered at any point). I think the system at least gives reasonable guarantees for shuffling near the mode of the length distribution. So the problem is likely more specifically the tail of the distribution.

In order for the rare buckets out there in the tail of the distribution to ever see enough data to be filled, pool_size needs to be increased to compensate. This would have a memory impact.
If pool_size is not increased, then these outlier buckets will just gather dust and never be used?

That's not easy to pre-compute

It doesn't need to be exact. A warmup step that reads the first 1k--10k lines of the file and estimates the mode from that would probably be good enough.

and requires different parameters per language or per task.

Yes, this would need to happen per task. Aggregating over languages would be tricky and not worth it.

try to automate that in the config config while reading the corpora length

We could compute this during config-config instead of in the beginning of training.
However, this makes config-config kind of mandatory to use, unless you want to estimate the parameter(s) yourself or hazard a guess. If there is only one parameter per task (the mode of the length), then this is still doable. For a full list of bucket sizes it is a lot to ask.

fit some random variable to the observed length distribution on a subset of the data
compute equiprobable bins through the cdf
use bins from 2. to define lists of bucket sizes,
store in a new task key

Yes, we could do it like this. This would be theoretically very elegant: the bins would be approximately equiprobable.
I'd just go with a rough approach where there are a few small (maybe size 1) bins around the mode and everything else gets chucked in a collector bin (or a few of those, if we want a little bit more granularity). This would not be very principled, but can be defined based on just one parameter.

We could obviously skip 1 and 2 and rely on hard observed data if we compute the full stats of each corpus.

It would be overkill to use the whole corpus to compute this.

If on the other we only care about ensuring that tail items are lumped in a single bucket, then I would consider just defining an outlier bucket by cutting off anything below an expected max length (say 128 or less), which can already be done by setting a lower n_bucket value.

I guess this also does the trick. It is the longer side that has the long tail, on the shorter side there is a minimum value that is quickly reached.

In the present 2D case, we could also rely on length ratios to define how many of the offset diagonals we tolerate before sending an example to the outlier bucket.

Not sure I follow.

There is a tradeoff between minimizing the amount of padding and minimizing the amount of correlation between examples in a batch.

If we're stressed out about wrapping around the full corpus and constructing batches with very similar examples (which is justifiable for LR langs in my opinion), then we can

remove the infinite cycling from the dataset,
use a flag to mark when the example stream has been exhausted
restart the iteration over the example stream when we've hit self.is_empty()

Wouldn't this just throw away the outliers entirely, instead of collecting them to (rare) outlier batches with (potentially several copies of) just the outliers?

In order for the rare buckets out there in the tail of the distribution to ever see enough data to be filled, pool_size needs to be increased to compensate. This would have a memory impact.
If pool_size is not increased, then these outlier buckets will just gather dust and never be used?

In the 1D impl currently on main, anything with a length above n_buckets gets chucked into the last bucket. so self._buckets[-1] is already an outlier bucket

It doesn't need to be exact. A warmup step that reads the first 1k--10k lines of the file and estimates the mode from that would probably be good enough.
[...] this makes config-config kind of mandatory to use, unless you want to estimate the parameter(s) yourself or hazard a guess.

Reading 10k more lines would slow down the initialization. For simplicity, I'd consider using pool_size items for the warmup, and offloading bucket definition in the init function of the LAB. If we also offload bucket definition to the LAB itself then we don't need to overcomplicate the config either.
If so, I would suggest implementing something like:

starting by computing an exact length-quantized bucket arrays
define a merge operation of buckets b1, b2  so that they share a ref to the same list
find the two most underused adjacent buckets and merge them
repeat until we get to the target number of buckets

cons:

It's a perf hit on startup
It's a mess to save the structure when resuming training (you also need the merge info)
kind of a mess to do weighted sampling (we need to drop duplicate refs first, or sample from the merge info)
pretty opaque for outsider devs

pros:

makes the config config optional again
simplifies the config definition to something humanly readable
easy to adapt the current behavior (the shared bucket ref does the rerouting)

If on the other we only care about ensuring that tail items are lumped in a single bucket, then I would consider just defining an outlier bucket by cutting off anything below an expected max length (say 128 or less), which can already be done by setting a lower n_bucket value.

I guess this also does the trick. It is the longer side that has the long tail, on the shorter side there is a minimum value that is quickly reached.

This is the simpler option, yes...

In the present 2D case, we could also rely on length ratios to define how many of the offset diagonals we tolerate before sending an example to the outlier bucket.

Not sure I follow.

In the current version, we define buckets for (src_len, tgt_len) tuples. What happens if the target has an outlier length but source is in the mode?

There is a tradeoff between minimizing the amount of padding and minimizing the amount of correlation between examples in a batch.

If we're stressed out about wrapping around the full corpus and constructing batches with very similar examples (which is justifiable for LR langs in my opinion), then we can

remove the infinite cycling from the dataset,
use a flag to mark when the example stream has been exhausted
restart the iteration over the example stream when we've hit self.is_empty()

Wouldn't this just throw away the outliers entirely, instead of collecting them to (rare) outlier batches with (potentially several copies of) just the outliers?

no, you'd keep creating batches until self._is_empty(), yield those final batches, and then you'd restart the dataset iteration.  Nothing lost, except the last batch might be smaller than expected.

can already be done by setting a lower n_bucket value

Ok. After clearing up some of my misunderstandings, I guess we could just lower the n_bucket value, so that the "high granularity" area contains a reasonable number when we go to 2D. Then everything else would either go into a single outlier bucket, or we could have e.g. 3: {source|target|both} exceeded the max length.

no, you'd keep creating batches until self._is_empty(), yield those final batches, and then you'd restart the dataset iteration. Nothing lost, except the last batch might be smaller than expected.

This also sounds great.
Pick whichever solution you prefer.

Ok. After clearing up some of my misunderstandings, I guess we could just lower the n_bucket value, so that the "high granularity" area contains a reasonable number of buckets when we go to 2D.

That was the idea, yes

Then everything else would either go into a single outlier bucket, or we could have e.g. 3: {source|target|both} exceeded the max length.

I guess having everything type of outlier into a single bucket would make sense, but i don't really know how to make this work with the spiralling bucket search pattern. Let me think on it.

Pick whichever solution you prefer.

They're not mutually exclusive, we can implement both.
And we're dropping the over-engineered solution for equiprobable bins, correct?
Do we want that as a rc2 feature, along with issues #8, #17, #24 and #25 ?

Some further comments:

there was a bug in the spiralling pattern. This should be fixed, but I definitely need to write unittests as well
there is a possibility that the re-initialization of the example stream and the refurbishing of the buckets end up taking quite some time. Hopefully the comm will be resilient enough if the data pipe ends up too slow, otherwise we might want to fiddle with the batch queue prefetching
I ended up removing self._lens and just calling len(bucket) as appropriate instead, to avoid any discrepancy

I'm also strongly considering using this PR to also give a somewhat less nondescript name to the DynamicDatasetIterator, which does not define the dynamic iteration process nor directly iterates over datasets (a multiplexer of streams of batches from tasks? TaskBatchMux makes for a very fun tongue-twister at the very least).

smoke tested on mahti.


How to use this on Multiple textboxes?

How to use this control for multiple textboxes. If i do the same, same text typed in all text boxes.

I guess it's more theoretical question regarding WPF that this particular control so I added labels. I hope somebody can take it and help you woth it.

There are no activity here hence I'll close it.


Added Instructions for Manual Testing for Notes feature in DG

Coverage remained the same at 57.438% when pulling 022934809f5a4f9a19b83b1a7a796ae99ca90eee on jomcruz93:master into 3d04d2ff656733d33d27cffa53f64efca6494c7a on CS2113-AY1819S1-T16-1:master.


(SERVER-119) Properly sign puppet extension values

It'd be great if the commit message for 4943604 contained at least the following information:  The behavior of the system without the patch applied, why this is a problem, and how the change resolves the problem.

Testing hipchat webhook...

I'm :+1: on this barring @jeffmccune's comment about the commit message. My knowledge of this is limited, however, so someone else should definitely take a look at it.

I've got no other comments either, I'm :+1: except the housekeeping comment.
Looks good.

If ya'lls ready to merge, let me know and I can amend that commit message before merge


Allow param and workspace propagation in Pipeline and Tasks

Feature request (or a Bug related to param and workspace propagation)

TEP0107 introduced parameter propagation between PipelineRuns --> Pipelines and TaskRuns --> Tasks.
However, it missed it in parameter propagation within a Pipeline (i.e. Pipeline --> PipelineTasks --> Steps). We should be able to propagate params within the same document for a more consistent workflow.
Use case

Reduction in verbosity.

Currently this (param propagation in a pipelinerun) works:
apiVersion: tekton.dev/v1
kind: PipelineRun
metadata:
  name: param-example
spec:
  params:
    - name: p1
      value: p1
    - name: p2
      value: p2
    - name: p3
      value: p3
  pipelineSpec:
    tasks:
      - name: t1
        taskSpec:
          steps:
            - image: ubuntu
              command: ["echo"]
              args: ["$(params.p1)"]
      - name: t2
        taskSpec:
          steps:
            - image: ubuntu
              command: ["echo"]
              args: ["$(params.p2)"]
      - name: t3
        taskSpec:
          steps:
            - image: ubuntu
              command: ["echo"]
              args: ["$(params.p3)"]

But this (param propagation within a pipeline) does not work:
apiVersion: tekton.dev/v1
kind: Pipeline
metadata:
  name: param-example
spec:
  params:
    - name: p1
    - name: p2
    - name: p3
  tasks:
    - name: t1
      taskSpec:
        steps:
          - image: ubuntu
            command: ["echo"]
            args: ["$(params.p1)"]
    - name: t2
      taskSpec:
        steps:
          - image: ubuntu
            command: ["echo"]
            args: ["$(params.p2)"]
    - name: t3
      taskSpec:
        steps:
          - image: ubuntu
            command: ["echo"]
            args: ["$(params.p3)"]

I'm inclined to treat this as a bug rather than a new feature since it is inconsistent when switching between in-lined and remote pipelines. The same also applies to Tasks and TaskRuns.
/kind bug

@tektoncd/core-collaborators WDYT?

Agreement from API WG on Monday 13th that this was an oversight and should be fixed!


[BE] Postcoordination action at class creation

When we create a new class, an event should be triggered , processed by postcoordination service, to create the equivalent class inside postcoordination as well. The default values should be :
NOT_SET for main linearization
DEFAULT for telescopic.
Scales will not be created.

Related to #41


Can not install ...

Description of the problem:
Cannot Install <package>
Unfortunately, there are a few different reasons a program might not install correctly.
Common reasons:
There are several different reasons a package might not install on your system.
Requires elevation (are you running in "admin" mode?)
Requires another program (some IDEs require the language to be installed)
Reporting:
The issue is likely to be related either to the winget.exe client, or the installer provided. If you would like to submit a bug, please put it in the client repository https://github.com/microsoft/winget-cli/issues

Unable to install KeePass, error is "incorrect parameters".

The User vs System https://github.com/microsoft/winget-cli/issues/149 and Dependencies https://github.com/microsoft/winget-cli/issues/163 features are on the client backlog. This is a duplicate of those issues.

The User vs System https://github.com/microsoft/winget-cli/issues/149 and Dependencies https://github.com/microsoft/winget-cli/issues/163 features are on the client backlog. This is a duplicate of those issues.


Glich in start up mode programm[affinity mask]180000 End trace4.19-191-245245370.].]end problem find bug

Find the issue and execute source

I think the issue is people don't know how to mind there step


Simplified backtraces

Objective
Remove Bevy internals from backtraces
Solution
Executors insert __rust_begin_short_backtrace into the callstack before running a system.

Example current output
thread 'Compute Task Pool (3)' panicked at src/main.rs:7:33:
Foo
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/panicking.rs:72:14
   2: foo::main::{{closure}}
             at ./src/main.rs:7:33
   3: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/ops/function.rs:294:13
   4: <Func as bevy_ecs::system::function_system::SystemParamFunction<fn() .> Out>>::run::call_inner
             at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/system/function_system.rs:661:21
   5: <Func as bevy_ecs::system::function_system::SystemParamFunction<fn() .> Out>>::run
             at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/system/function_system.rs:664:17
   6: <bevy_ecs::system::function_system::FunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run_unsafe
             at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/system/function_system.rs:504:19
   7: bevy_ecs::schedule::executor::multi_threaded::ExecutorState::spawn_system_task::{{closure}}::{{closure}}
             at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs:621:26
   8: core::ops::function::FnOnce::call_once
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/ops/function.rs:250:5
   9: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/panic/unwind_safe.rs:272:9
  10: std::panicking::try::do_call
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:554:40
  11: __rust_try
  12: std::panicking::try
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:518:19
  13: std::panic::catch_unwind
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panic.rs:142:14
  14: bevy_ecs::schedule::executor::multi_threaded::ExecutorState::spawn_system_task::{{closure}}
             at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs:614:23
  15: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::future::future::Future>::poll
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/panic/unwind_safe.rs:297:9
  16: <futures_lite::future::CatchUnwind<F> as core::future::future::Future>::poll::{{closure}}
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-lite-2.2.0/src/future.rs:588:42
  17: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/panic/unwind_safe.rs:272:9
  18: std::panicking::try::do_call
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:554:40
  19: __rust_try
  20: std::panicking::try
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:518:19
  21: std::panic::catch_unwind
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panic.rs:142:14
  22: <futures_lite::future::CatchUnwind<F> as core::future::future::Future>::poll
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-lite-2.2.0/src/future.rs:588:9
  23: async_executor::Executor::spawn::{{closure}}
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-executor-1.8.0/src/lib.rs:158:20
  24: async_task::raw::RawTask<F,T,S,M>::run::{{closure}}
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-task-4.7.0/src/raw.rs:550:21
  25: core::ops::function::FnOnce::call_once
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/ops/function.rs:250:5
  26: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/panic/unwind_safe.rs:272:9
  27: std::panicking::try::do_call
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:554:40
  28: __rust_try
  29: std::panicking::try
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:518:19
  30: std::panic::catch_unwind
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panic.rs:142:14
  31: async_task::raw::RawTask<F,T,S,M>::run
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-task-4.7.0/src/raw.rs:549:23
  32: async_task::runnable::Runnable<M>::run
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-task-4.7.0/src/runnable.rs:781:18
  33: async_executor::Executor::run::{{closure}}::{{closure}}
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-executor-1.8.0/src/lib.rs:254:21
  34: <futures_lite::future::Or<F1,F2> as core::future::future::Future>::poll
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-lite-2.2.0/src/future.rs:449:33
  35: async_executor::Executor::run::{{closure}}
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-executor-1.8.0/src/lib.rs:261:32
  36: futures_lite::future::block_on::{{closure}}
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-lite-2.2.0/src/future.rs:99:19
  37: std::thread::local::LocalKey<T>::try_with
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/thread/local.rs:286:16
  38: std::thread::local::LocalKey<T>::with
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/thread/local.rs:262:9
  39: futures_lite::future::block_on
             at /home/vj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-lite-2.2.0/src/future.rs:78:5
  40: bevy_tasks::task_pool::TaskPool::new_internal::{{closure}}::{{closure}}::{{closure}}::{{closure}}
             at /home/vj/workspace/rust/bevy/crates/bevy_tasks/src/task_pool.rs:180:37
  41: std::panicking::try::do_call
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:554:40
  42: __rust_try
  43: std::panicking::try
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:518:19
  44: std::panic::catch_unwind
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panic.rs:142:14
  45: bevy_tasks::task_pool::TaskPool::new_internal::{{closure}}::{{closure}}::{{closure}}
             at /home/vj/workspace/rust/bevy/crates/bevy_tasks/src/task_pool.rs:174:43
  46: std::thread::local::LocalKey<T>::try_with
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/thread/local.rs:286:16
  47: std::thread::local::LocalKey<T>::with
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/thread/local.rs:262:9
  48: bevy_tasks::task_pool::TaskPool::new_internal::{{closure}}::{{closure}}
             at /home/vj/workspace/rust/bevy/crates/bevy_tasks/src/task_pool.rs:167:25
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Encountered a panic in system `foo::main::{{closure}}`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!
get on your knees and beg mommy for forgiveness you pervert~ 💖

Example output with this PR
Panic at src/main.rs:7:33:
Foo
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/panicking.rs:72:14
   2: foo::main::{{closure}}
             at ./src/main.rs:7:59
   3: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
             at /rustc/8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6/library/core/src/ops/function.rs:294:13
   4: <Func as bevy_ecs::system::function_system::SystemParamFunction<fn() .> Out>>::run::call_inner
             at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/system/function_system.rs:661:21
   5: <Func as bevy_ecs::system::function_system::SystemParamFunction<fn() .> Out>>::run
             at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/system/function_system.rs:664:17
   6: <bevy_ecs::system::function_system::FunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run_unsafe
             at /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/system/function_system.rs:504:19
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Encountered a panic in system `foo::main::{{closure}}`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!

Full backtraces (RUST_BACKTRACE=full) are unchanged.
Alternative solutions
Write a custom panic hook. This could potentially let use exclude a few more callstack frames but requires a dependency on backtrace and is incompatible with user-provided panic hooks.

Changelog

Backtraces now exclude many Bevy internals (unless RUST_BACKTRACE=full is used)

Could you please ensure this is optional, I have my own panic handler =)

Now that bevy_dev_tools has been merged (#11341), can any part of this be moved there? I'm not sure I want bevy_ecs depending on bevy_dev_tools, but we should at least consider it.

What does the backtraces look like when there's a panic in the render world?

What does the backtraces look like when there's a panic in the render world?

They're still not great. git apply this, then run RUST_BACKTRACE=1 cargo run --example lighting to see the backtrace.
diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs
index d972356b0..c4e682d42 100644
--- a/crates/bevy_render/src/lib.rs
+++ b/crates/bevy_render/src/lib.rs
@@ -438,6 +438,7 @@ unsafe fn initialize_render_app(app: &mut App) {
                 )
                     .in_set(RenderSet::Render),
                 World::clear_entities.in_set(RenderSet::Cleanup),
+                || panic!("Render world panic!"),
             ),
         );

Is there any way we can skip printing the second panic?

Unless we manipulate it using backtraces, that's likely going to show up for every level of std::panic::catch_unwind we have.

Is there a way around that? Otherwise I'm not sure if that change is worth it.

Perhaps AppExit can be modified to be AppExit(ExitCode), but that's probably worth it's own issue and PR.


Make header links work on site & github

This PR makes it so that all subsection headers (a ## that creates an <h2>) use lowercase letters. There is a little bit of discussion about this in #5197, but I'll explain everything below.
The way that GitHub adds ids to headers converts all of the letters to lowercase. The current way that the React Router website adds ids to headers is to convert to lowercase unless the header is from the api section, in which case it preserves the case for all letters.
For example, on GitHub, the documentation for the section on a <Route>'s render methods has the id route-render-methods. However, on the website, it has the id Route-render-methods.
For hrefs generated by the react-router-website's markdown-loader, there is no issue with this case discrepancy.  The problem occurs when there is an existing link within one of the markdown files. If these links are in all lowercase, they will work on GitHub but fail on the React Router website, and vice versa.
I've avoided the main headers (#/<h1>) because I don't think that we need to worry about them (we can just link to the whole markdown file instead of the header). I think that there is a separate issue there (try clicking the <Router> link in the first sentence of the <MemoryRouter> documentation), but that can be handled in a different PR.

Thanks, @pshrmn. Your attention to detail is very much appreciated :)


Performing hot restart... during flutter run or debug

Steps to Reproduce

...When I modified the code click save
...The system keeps prompting "Performing hot restart..."

Expected results: Able to perform hot reload normally

Hi @yangchen1993, do you also have this issue when you use either Android Studio or the flutter run tool?

Hi @yangchen1993, do you also have this issue when you use either Android Studio or the flutter run tool?

yes,even I use terminal.It's not about tools

is this ok?

We need to gather some more information.
Kindly create a dummy flutter project.
Then run flutter run -v
Make some changes and save the file then try to hot restart.
The output of all this is going to be very large, so kindly put it into a .txt file and attach it here.
Alternatively, you could run flutter run -v > log.txt, but you'll have to be monitoring the file for changes and that may not be convenient for you.

I know the problem,It's because when i use chrome to set the device to iphone,Appears suddenly while in use: “Error: Bad state: Not connected to an application.”

ok,i get it.it's about navigate.userAgent.thank u

Have you found the cause of the issue?


hotkey to open main window after minimized to tray or minimized in general (ctrl shift space)

done


GSLB_Pool_Member_A and _AAAA: virtualServer unexpected behaviour

Environment

Application Services Version: f5-appsvcs-3.21.0-4.noarch
BIG-IP Version: <IP_ADDRESS>

Summary
Hi
I find that documentation's example and the description of "GSLB_Pool_Member_A" and "_AAAA" in the schema itself could explain better the use of the element virtualServer
                "members": [
                    {
                        "ratio": 10,
                        "server": {
                            "use": "/Common/Shared/testServer"
                        },
                        "virtualServer": "0"
                    }
                ],

If I try using an index, AS3 returns: "01070226:3: Pool Member 0 references a nonexistent Virtual Server"
See full example attached.
If instead I use the name in the virtualServer element:
        "members": [
          {
            "server": {
              "use": "/Common/Shared/onprem_default"
            },
            "virtualServer": "https_account_onprem_default",
            "ratio": 50
          }
        ],

It works and creates the expected GTM configuration:
gtm pool a Application/pool_account_f5bddemos_io {
    members {
        /Common/onprem_default:https_account_onprem_default {
            member-order 0
            ratio 50
        }
    }
    partition gslb_f5bddemos_io
}

I'm not sure if this is just a documentation issue or an actual bug.
IMHO virtualServer should be pointer to a virtualServer but the schema doesn't (it's just a string, no schema checking?) and the actual description of the schema suggests the string has a boolean behavior ... Very confusing altogether.
Any clarifications will be greatly appreciated.
Thanks
Steps To Reproduce
See attached files AS3 declarations, the first one indicating the virtualServer by name (works) and the second one by number (doesn't work)
bigip-gslb-update-config.using-name.json.txt
bigip-gslb-update-config.using-number.json.txt

The virtualServer property is a string because you are selecting one of the virtual servers from the GSLB Server that is being referenced by a pointer. I do agree that the description is unclear, and could be improved.
Now, on to the number versus name issue. This is a result of a feature evolving over time, and the documentation not evolving well with it. Originally, you could not specify names on the GSLB Server's virtual servers. They were simply given a name based on their index in the list of virtual servers. We later added the name property to allow users to overwrite this behavior. When referencing a virtual server by number, you are really trying to reference it by its default name.
In short, reference by name, but use numbers if you did not specify names on your virtual servers.

@dstokesf5 could this be classified with the doc tag?

I have added AUTOTOOL-1892 to our internal product backlog to improve the documentation around this functionality.

It has been resolved.


Error screen popup

Microsoft PowerToys version
0.51.0
Installation method
PowerToys auto-update
Running as admin
No
Area(s) with issue?
General
Steps to reproduce
The popup occurred while I was uninstalling MAVLink. There is also an update of Adobe Camera Raw running in the background. I am unsure if any of this is related. I was not actively using PowerToys or any of its utilities at the time.
2024-01-28.txt
✔ Expected Behavior
Nothing
❌ Actual Behavior
An error screen popup
Other Software
MAVProxy?

System.Collections.Generic.KeyNotFoundException: The given key 'Path' was not present in the dictionary.
   Source: System.Private.CoreLib
   TargetAssembly: System.Private.CoreLib, Version=<IP_ADDRESS>, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
   TargetModule: System.Private.CoreLib.dll
   TargetSite: Void ThrowKeyNotFoundException[T](T)
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at PowerLauncher.Helper.EnvironmentHelper.GetMergedMachineAndUserVariables(Dictionary`2 environment)
   at PowerLauncher.Helper.EnvironmentHelper.<>c.<UpdateEnvironment>b__3_0()
   at Wox.Infrastructure.Stopwatch.Normal(String message, Action action)
   at PowerLauncher.Helper.EnvironmentHelper.UpdateEnvironment()
   at PowerLauncher.MainWindow.ProcessWindowMessages(IntPtr hwnd, Int32 msg, IntPtr wparam, IntPtr lparam, Boolean& handled)
   at System.Windows.Interop.HwndSource.PublicHooksFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)

This bug has been fixed in 0.72.0
Please update PowerToys to the latest version: https://github.com/microsoft/PowerToys/releases/tag/v0.77.0
/dup #25848


Podman in k8s

This merge request is a possible implementation for jupyterhub/binderhub#1513.
It requires an image built with the changes I proposed on manics/repo2podman#32 (currently available at docker.io/sgaist/repo2podman).
It adds a new optional pod that serves as a build host for podman which allows to keep a cache for the layers/images on the host machine in a similar fashion to the current dind implementation.
The current implementation uses the runAsUser security context value set to the podman user which means that the host folder shall have corresponding ownership in order for podman to write in there.
The configuration example assumes the use of a BinderHub image that contains the extra environment variables patch applied. It can be done with an official image by modifying the custom class in the extra_config field and add there an implementation of the submit function with the two additional variables added to the env variable.
Just in case, the daemonset name is the acronym of "podman in kubernetes" hence pink.
Configuration example:
service:
  type: ClusterIP

config:
  BinderHub:
    debug: true
    hub_url: "https://my.host.com/jupyter"

    use_registry: true
    image_prefix: "my.registry.com/my-project/my-image-"

    build_docker_host: "unix:///local/fake_var/run/pink/docker.sock"
    build_image: "docker.io/sgaist/repo2podman:latest"

pink:
  enabled: true
  daemonset:
    image:
      name: docker.io/sgaist/repo2podman
  hostStorageDir: /local/fake_var/lib/pink/storage/
  hostSocketDir: /local/fake_var/run/pink/

imageCleaner:
  host:
    dockerSocket: /local/fake_var/run/pink/docker.sock
    dockerLibDir: /local/fake_var/lib/pink/storage

ingress:
  enabled: true
  ingressClassName: nginx
  hosts:
    - my.host.com

extraConfig:
  01-Repo2Podman: |
    class Repo2PodmanBuild(Build):
        extra_envs = {
            "CONTAINER_HOST": "unix:///var/run/docker.sock",
            "REGISTRY_AUTH_FILE": "/root/.docker/config.json",
        }

        def get_r2d_cmd_options(self):
            return ["--engine=podman"] + super().get_r2d_cmd_options()

    c.BinderHub.build_class = Repo2PodmanBuild

Small note, as it is a preliminary work I have not written the documentation related to it yet.

Following this month's JupyterHub's meeting, @manics and myself continued a bit on the podman front discussing the podman engine for repo2docker as well as this patch content. One thing that I was not aware of was that beside podman being a drop in replacement for docker in terms of command line interface, it now also implements a docker compatible API from a service point of view.
Therefore I ran a BinderHub deployment test with the pink daemonset enabled, the standard Build class, and the image cleaner configured to watch the storage folder of podman.
All these together worked quite nicely.
This means that the environment variable addition is not strictly required for the goal of using podman in place of docker for the pod building the images. However it does allow people to choose whether they want to go full podman, full docker, or a mix of both as it would be the default.

The diff between the dind and pink K8s templates is fairly small
diff --color -Nur helm-chart/binderhub/templates/dind/daemonset.yaml helm-chart/binderhub/templates/pink/daemonset.yaml
--- helm-chart/binderhub/templates/dind/daemonset.yaml	2021-09-18 20:57:59.971717855 +0100
+++ helm-chart/binderhub/templates/pink/daemonset.yaml	2022-09-23 13:29:38.484322020 +0100
@@ -1,20 +1,20 @@
-{{- if .Values.dind.enabled -}}
+{{- if .Values.pink.enabled -}}
 apiVersion: apps/v1
 kind: DaemonSet
 metadata:
-  name: {{ .Release.Name }}-dind
+  name: {{ .Release.Name }}-pink
 spec:
   updateStrategy:
     type: RollingUpdate
   selector:
     matchLabels:
-      name:  {{ .Release.Name }}-dind
+      name:  {{ .Release.Name }}-pink
   template:
     metadata:
       labels:
-        name: {{ .Release.Name }}-dind
+        name: {{ .Release.Name }}-pink
         app: binder
-        component: dind
+        component: dind  # Lying to build so current affinity rules can work
         release: {{ .Release.Name }}
         heritage: {{ .Release.Service }}
     spec:
@@ -28,47 +28,59 @@
         operator: Equal
         value: user
       nodeSelector: {{ .Values.config.BinderHub.build_node_selector | default dict | toJson }}
-      {{- with .Values.dind.initContainers }}
+      {{- with .Values.pink.initContainers }}
       initContainers:
         {{- . | toYaml | nindent 8 }}
       {{- end }}
+      {{- with .Values.pink.daemonset.image.pullSecrets }}
+      imagePullSecrets:
+        {{- . | toYaml | nindent 8 }}
+      {{- end }}
+      securityContext:
+        fsGroup: 1000
+
       containers:
-      - name: dind
-        image: {{ .Values.dind.daemonset.image.name }}:{{ .Values.dind.daemonset.image.tag }}
+      - name: pink
+        image: {{ .Values.pink.daemonset.image.name }}:{{ .Values.pink.daemonset.image.tag }}
+        imagePullPolicy: {{ .Values.pink.daemonset.image.pullPolicy }}
+
         resources:
-          {{- .Values.dind.resources | toYaml | nindent 10 }}
+          {{- .Values.pink.resources | toYaml | nindent 10 }}
         args:
-          - dockerd
-          - --storage-driver={{ .Values.dind.storageDriver }}
-          - -H unix://{{ .Values.dind.hostSocketDir }}/docker.sock
-          {{- with .Values.dind.daemonset.extraArgs }}
-          {{- . | toYaml | nindent 10 }}
-          {{- end }}
+          - podman
+          - system
+          - service
+          - --time=0
+          - unix:///var/run/pink/docker.sock
         securityContext:
           privileged: true
+          runAsUser: 1000  # podman default user id
+
         volumeMounts:
-        - name: dockerlib-dind
-          mountPath: /var/lib/docker
-        - name: rundind
-          mountPath: {{ .Values.dind.hostSocketDir }}
-        {{- with .Values.dind.daemonset.extraVolumeMounts }}
+        - mountPath: /home/podman/.local/share/containers/storage/
+          name: podman-containers
+        - mountPath: /var/run/pink/
+          name: podman-socket
+
+        {{- with .Values.pink.daemonset.extraVolumeMounts }}
         {{- . | toYaml | nindent 8 }}
         {{- end }}
-        {{- with .Values.dind.daemonset.lifecycle }}
+        {{- with .Values.pink.daemonset.lifecycle }}
         lifecycle:
           {{- . | toYaml | nindent 10 }}
         {{- end }}
       terminationGracePeriodSeconds: 30
       volumes:
-      - name: dockerlib-dind
+      - name: podman-containers
         hostPath:
-          path: {{ .Values.dind.hostLibDir }}
-          type: DirectoryOrCreate
-      - name: rundind
+          path: {{ .Values.pink.hostStorageDir }}
+          type: Directory
+      - name: podman-socket
         hostPath:
-          path: {{ .Values.dind.hostSocketDir }}
-          type: DirectoryOrCreate
-      {{- with .Values.dind.daemonset.extraVolumes }}
+          path: {{ .Values.pink.hostSocketDir }}
+          type: Directory
+
+      {{- with .Values.pink.daemonset.extraVolumes }}
       {{- . | toYaml | nindent 6 }}
       {{- end }}
 {{- end }}

so a couple of alternative options are:
Parameterise the dind templates:

Make .Values.dind.securityContext fully configurable
Allow the dind volumeMounts to be overridden (or perhaps have an enable/disable switch, and fully configure podman through .Values.dind.daemonset.extraVolumeMounts and .Values.dind.daemonset.extraVolumes)

Add conditionals to the dind templates, e.g.
        args:
       {{- if eq .Values.containerBuilderPod "dind" }}
          - dockerd
          - --storage-driver={{ .Values.dind.storageDriver }}
          - -H unix://{{ .Values.dind.hostSocketDir }}/docker.sock
       {{- end }}
       {{- if eq .Values.containerBuilderPod "pink" }}
          - podman
          - system
          - service
          - --time=0
          - unix:///var/run/pink/docker.sock
       {{- end }}
        {{- with .Values.dind.daemonset.extraArgs }}
        {{- . | toYaml | nindent 10 }}
        {{- end }}

If we decide on this route I think deprecating .Values.dind.enabled and replacing it with something like .Values.containerBuilderPod will be more extensible in the future.

I like the idea of a configurable containerBuilderPod
Should we also consider modifying the dind label used to keep build pods on the same node that the builder pod itself ?

Maybe we could do this in two PRs? This one for getting podman to work (so a configurable containerBuilderPod with no breaking changes). Then a separate PR for renaming the dind label to something more generic, since it's an internal breaking change- in fact a PR to change the label could be opened now, since it'll be a fairly minor conflict to resolve, and means others can agree/disagree!

As per your excellent idea I merge the two templates into a "new one" that covers both cases and should be easy to extend further.
I haven't touched yet the securityContext part as I wanted to first ensure that the new template would fit the bill.

@manics I think it's a good idea to do the Pod Security Policy work in a separate PR. With the replacement of them in K8s 1.25 for the Pod Security admission controller, it might be better to concentrate on the latter.

@consideRatio Is anything else needed here before merging?

Thanks @manics for the ping, @manics are your unresolved comments ready be resolved, or do they need to be followed up further?

When using pink with it's default settings it's also necessary to override the imageCleaner config.
imageCleaner:
  host:
    dockerSocket: /var/run/pink/podman.sock
    dockerLibDir: /var/lib/pink/storage

Since we're now making a breaking change in this PR could we get rid of these properties and use whatever is configured in dind/pink?
I've also just noticed dind uses hostLibDir for it's storage directory whereas pink uses hostStorageDir. @consideRatio do you think we should keep this as it represents the underlying container implementation, or use the same property name as an abstraction?

@consideRatio do you think we should keep this as it represents the underlying container implementation, or use the same property name as an abstraction?

Hmm hmm I'm feeling a bit out of my depth here and don't feel ready to opine about it. I think your judgement call will be acceptable no matter what though!

When using pink with it's default settings it's also necessary to override the imageCleaner config.
imageCleaner:
  host:
    dockerSocket: /var/run/pink/podman.sock
    dockerLibDir: /var/lib/pink/storage

Since we're now making a breaking change in this PR could we get rid of these properties and use whatever is configured in dind/pink?

But there was a local situation as well, or? Or is the imageCleaner stuff only activated as part of using dind/pink? I think if imageCleaner is tightly coupled to the use of dind/pink, then relying on configuration under there is the right call.

Funny you bring imageCleaner on as I was about to ask some questions for it.
To the best of my comprehension, the default deployment for BinderHub would be using the host Docker and a host configured cleaner.
Then you have the option to use dind and that brings two cases to consider:

Enabling dind and configure imageCleaner host's paths to point to the dind socket and lib folder
Enabling dind and disable imageCleaner's host configuration

The former works because everything happens on the same node and thus all the paths are valid on the hosts. However there's a side effect. There are two containers created in the imageCleaner pod, one for the host and one for dind which, in the end, are checking the same volumes.
In the second case, only the container watching dind is created.
One thing we could do is refactor the cleaner template logic to follow a pattern similar to the builder daemonset and thus ensure that we have only one container doing the watching and cleanup.
One thing for that that we could do is to remove the loop in the containers part of the template.
Maybe rename local to host in the enumeration defined for imageBuilderType ?

Thanks for thinking about this @sgaist!
When using local, is image building done by the binderhub software in the binder pod - using a local docker daemon? Ideally, the configuration value should clarify that directly from its name. If we update this name, I'd appreciate if we make the name verbose enough to help a person understand more from the name directly.
I think local means binder_pod_local_docker_daemon and that the binder pod is mounted a docker daemon from the host or similar, do you think that is that correct @sgaist?
Ah, but I get your point about using host over local due to this:
https://github.com/jupyterhub/binderhub/blob/0049297e69dfbc93a6a1079ef85c2b898ebad192/helm-chart/binderhub/templates/image-cleaner.yaml#L38-L41
I think as long as we have imageCleaner's terminology of host, it makes sense to speak of host instead of local. If you want to change that would be fine in my mind.
Confusion points are:

The docker sockets in image cleaner pods and binder pods are mounted from hostPath volumes it seems, "local" and "dind".
The image cleaner pods are being created by a daemonset, so they will run on every node - not just where the binder pod runs.

Oh I get some clarity now! If a node has been running in k8s for a long time, and pulls more and more images because its starting more and more pods of different kinds - it can run out of space. So, image cleaning on the host is not relevant only for where you are building pods using some docker daemon. It can be directly relevant just by being a node that starts a lot of pods.
At the same time, we have this k8s official description, saying that its not recommended to have external tools to do such things: https://kubernetes.io/docs/concepts/architecture/garbage-collection/#containers-images. Hmmm... I think removal of the imageCleaner host option is something worth considering, but its beyond the scope of this PR to consider I think.

@sgaist / @manics, I know ~nothing about Podman. Should it be exposed to image cleaning, and if so, can it be exposed to image cleaning by our current system running https://github.com/jupyterhub/docker-image-cleaner?

For the purposes of building and storing images podman is the same as Docker- if you build an image it's going to stay around until it's deleted. The socket interface presented by podman is the same as docker, that's why I wasn't bothered about renaming the socket to podman.sock- as far as the client is concerned (Docker cli, docker-py, repo2docker) it shouldn't matter, except that if the socket isn't mounted to /var/run/docker.sock the client needs configuration to use the alternative path.
How about we only use imageCleaner.host.* for the local docker host, and if dind/pink are enabled ignore those values and use dind.{hostSocketDir,hostLibDir} / pink.{hostStorageDir,hostSocketDir} instead, since AFAICT there's no reason to run the cleaner with any other paths?

I think we can simplify some things here indeed.
Do we have an agreement on the following for this patch:

Change local to host for the imageBuilderType enumeration
Refactor the image-cleaner daemonset so it only creates the container matching the imageBuilderType
Deprecate the use of host.enabled in the same manner as dind.enabled

?

Agreement!
/ Erik on mobile

Sounds fine to me as well, but given the length of this PR how does everyone feel about merging as is, and doing changes in a follow-up?

(Not to be blocking a merge as this can be done after merge)

@sgaist could you review the PR description a final time so that it doesn't include outdated descriptions of what this PR ended up doing?

Merging!

This broke the BinderHub CI upgrade test on main:
https://github.com/jupyterhub/binderhub/blob/fb773a0891d0ed84ad397ec4eeabec7b79e2947b/.github/workflows/test.yml#L186
Run . ./ci/common

Installing already released binderhub version 1.0.0-0.dev.git.2920.hc64d689
using old config
Error: INSTALLATION FAILED: execution error at (binderhub/templates/NOTES.txt:194:4):

#################################################################################
######   BREAKING: The config values passed contained no longer accepted    #####
######             options. See the messages below for more details.        #####
######                                                                      #####
######             To verify your updated config is accepted, you can use   #####
######             the `helm template` command.                             #####
#################################################################################

CHANGED:

dind:
  enabled: true

must as of version 0.3.0 be replace by

imageBuilderType: dind

On the bright side we've now tested the upgrade message :smiley:

Thanks for following up @manics!!

Thanks for the reviews guys !
@consideRatio I have updated the description and I think it's now complete.
@manics can you double check it in case I missed something ?

@sgaist thanks! Can you provide a leading paragraph, one or two sentence, to introduce what podman is in the PR description as well?
Relevant for the paragraph:

link to read more about podman
that it is similar to docker as mentioned by @manics in https://github.com/jupyterhub/binderhub/pull/1531#issuecomment-1351616863

@consideRatio Sure thing !
Done in three sentences but I can rework that if needed.

Perfect @sgaist thanks!


form.$error.required and form.$invalid validators are not syncing

I was wondering if this is an angular bug. The $invalid is undefined while my $error.required is true when a custom $validators is declared on a directive.
Check this example: http://plnkr.co/edit/NuDGB64IetpcsaVB03T7?p=preview

This seems like an Angular bug indeed. BTW, it has been browkn since v1.3.0-rc.1!

Oh ok, so that's why thanks for the response

After further investigation, I'm not sure if this is Angular's fault or your directive's.
There is definitely a bug in your directive, but I'm not sure if Angular should treat it differently - it's a gray, undocumented area :smiley:
Here's what happens:
In your custom validator, when modelValue is falsy (e.g. undefined, as happens initially), you return check without ever initializing it. You declare var check inside the if block, but since var is function scoped (not block scoped), the declaration gets hoisted to the top of the function block. Thus, when you return check, check is declared but not initialized (i.e. undefined).
This is obviously unintentional. You should change the validator to something like:
function (modelValue, viewValue) {
  var check = true;
  if (modelValue) {
    ...
  }
  return check;
}

Now, Angular normally expects a boolean (true or false) to be returned from a validator. Returning undefined triggers a special behavior though: It designates that the validation is pending and sets both $invalid and $valid to undefined. This behavior was probably intended (and is internally used) for async validators.
Returning undefined from a sync validator, tricks Angular into setting the valididty state as $pending (i.e. as happens with async validators).
--
Feature or Bug ?
I lean towards the latter. @Narretz,  thoughts ?
BTW, the fix would be to change the first line here, so that result is always a boolean, regardless of what the validator returns: var result = !!validator(modelValue, viewValue);
(But it might be a breaking change...)

Ok, I understand your point but since the $validator is 'available' as it is declared.
ngModel.$validators.available = function (modelValue, viewValue) {
}
It should have been only reflected on the $error.available and not the whole $invalid.. That is from my point of view. I'm aware that this can be solved by doing a little extra conditional in the directive like
if(ngModel.$error.required){
check = true;
}else{
check = false;
}
But  that's an additional manual coding. So as much as possible I  would like to avoid it. But thanks for the response, I don't know if it still considered as a bug though

It should have been only reflected on the $error.available and not the whole $invalid.

Not really. The $valid/$invalid refer to the validity of the control as a whole; i.e. taking all validators into account. If one validator (in this case available) has not determined yet if the value is valid or not (by returning undefined), then the whole validity can't be determined.
--

I'm aware that this can be solved by doing a little extra conditional in the directive like
if (ngModel.$error.required) { check = true; } else { check = false; }

Not sure what you mean. required doesn't have anything to do with this issue. This is about returning a boolean value from your validator.

This is what I meant Just check the code below and just take a look on the 2nd to 6th line that I have added.
ngModel.$validators.available = function (modelValue, viewValue) {
if(ngModel.$error.required){
check = false;
}else{
check = true;
}
if(modelValue){
var check = modelValue.split('-');
var y = parseInt(check[0], 10);
var m = parseInt(check[1], 10);
var d = parseInt(check[2], 10);
var date = new Date(y,m-1,d);
if (date.getFullYear() == y && date.getMonth() + 1 == m && date.getDate() == d) {
check = true;
} else {
check = false;
name = ngModel.$name;
if(datePatternCounter.indexOf(name) == -1){
datePatternCounter.push(name);
element.parent().siblings("span.errors").append(' * Invalid Date ');
}
}
					}

					return check;
				}

It's working great so far. You can take a loot at this plunker prevew: http://plnkr.co/edit/NuDGB64IetpcsaVB03T7?p=preview

I still don't understand. The required validity state is irrelevant here. You should just make sure you return a boolean value (whatever makes sense in your usecase) even if the modelValue has not been set.
Previously, you were returning undefined and that is what was causing issues.

Ok, understood. Thanks for the help!

Regarding your comment in https://github.com/angular/angular.js/issues/14734#issuecomment-224568626 @gkalpak about returning undefined from a sync validator, it's definitely not documented that you can do that. But the question is what should happen when you do it? The current behavior isn't ideal, but should we throw?

I think it makes sense to convet the value to boolean.
Note that we are already (sort of) doing this, when we determine the overall sync validators validity (here. But we are still setting the validity of the specific validator to undefined (which triggers the whole undocumented/unwanted behavior).
Essentially, I am proposing to change this line (which will only affect the behavior of that line) like this:
-var result = validator(modelValue, viewValue);
+var result = !!validator(modelValue, viewValue);

While writing up tests for the change, I noticed something possibly unexpected. Aside from the expected true -> true, false -> false, and undefined -> undefined, the following also exist:
null -> true
0 -> true
'' -> true
NaN -> true
Since all of these values are falsey, this change will actually alter their behaviour if used as the return values of validator functions. Of course, we only ever expected true or false from a validator, so this use case should be fairly minimal. Nonetheless, this will be a breaking change.

@BobChao87, do you have a PR or branch I can take a look at? What tests are you referring to exactly?

Hopefully that resolves any questions about what I was trying to describe. The changes are all in the negative tests.

Hi,
Because $error is object will have error if it is invalid.  while debugging
if we check $error is true.
Thanks,
Warm Regards :Brijesh Pal
Mobile : +919790752446
On Tue, Sep 27, 2016 at 1:56 PM, BobChao87<EMAIL_ADDRESS>wrote:

Hopefully that resolves any questions about what I was trying to describe.
The changes are all in the negative tests.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/angular/angular.js/issues/14734#issuecomment-249799388,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGltbJFIcEmF6kLzyvJGsNhGbUlV0J7lks5quNM7gaJpZM4IwvAH
.

@BobChao87, I see. This seems to be an unexpected side effect of an unintended use. Internally, the $setValidity treats values as follows:

undefined: Set validity to pending.
true: Set validity to true for the specified validator.
false: Set validity to false for the specified validator and the parent form/ngForm.
everything else: Clear the $error/$$success fields, which effectively sets the field's validity to true.

AFAICT, this last "feature" is intended for internal use only and specifically used with null. Since it was never documented that sync validators should return anything other than boolean values, I believe it is a totally reasonable breaking change.
(I only expect this to affect very very few people (if any) and even it's not our fault :smiley:)

Hmmm, okay, sounds good to me. I'll clean up the code and write up a breaking change note and get that in in the next couple of days. I agree, should be a very small cross-section of users that will be affected by this change.


Added support for setting the notifications flag for crashlytics

Useful for CI if don't want to spam users every time a new build is available.
http://support.crashlytics.com/knowledgebase/articles/370383-beta-distribution-with-ios-build-servers

Thanks so much for this, @lmirosevic. I'll cut a new release shortly.


fix: incorrect comment

Change incorrect comment in example code.
omit for guild command -> omit for global command
Documentation change checklist

[x] My documentation changes follow the docs style guide and any code examples follow the coding style guide.
[x] I tested that my change works before raising the PR (via running doxygen, and testing examples).
[x] I have not moved any existing pages or changed any existing URLs without strong justification as to why.
[x] I have not generated content using AI or a desktop utility such as grammarly.

thanks!


Dynamic module

Messy implementation of RBAC and Auth, requires refactor. See #47

Due to lack of  RBAC and Auth, dynamic module is unable to be implemented since backend is lacking the feature as well. This branch SHOULD NOT be merged, but only to be used as a reference on what not to do.


Correctly define ip_forward flag, not present by default in opts

As it is defined to be a boolean in the resource, the only possible
values allowed to the user are true or false. Let docker use its default
value instead of setting a default flag.
If the user knows what he's doing he'll be able to give a specific value
to the flag.

Thanks


💚 Pin ray to 2.5.1 for tests (avoid logging issue)

Codecov Report

Merging #61 (0392bd1) into main (d51c8a3) will decrease coverage by 0.06%.
Report is 2 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
- Coverage   92.56%   92.51%   -0.06%
==========================================
  Files           9        9
  Lines         148      147       -1
  Branches       13       13
==========================================
- Hits          137      136       -1
  Misses          6        6
  Partials        5        5

see 1 file with indirect coverage changes


Perform publish workflow on change to master

Instead of only trying to perform the publish workflow when the
package.json file changes, this makes it so that any change in master
results in an attempt to publish to NPM.
The initial limitation was to try and minimize the amount of unnecessary
work to be done, since anything besides bumping the version wouldn't
require a publish attempt.
However, by having it in place it becomes impossible to trigger a publish
if a fix to the publish process was made that doesn't affect the
package.json file.
This change I believe also goes more in line with the more streamline
approach we want to move towards. We are already relying on NPM to make
sure we don't publish if the version remained the same, so this just
goes all in.
Signed-off-by: Ruben Aguiar<EMAIL_ADDRESS>

The change makes sense. Processes that run more often as always better.
My main comment is regarding the PR description, I find there's too much.
The problem that this PR solves is the following:

However, by having it in place it becomes impossible to trigger a publish
if a fix to the publish process was made that doesn't affect the
package.json file.

Which causes side-effects. But the side-effects are gates but

We are already relying on NPM to make
sure we don't publish if the version remained the same, so this just
goes all in.

I would have written (What, Why):
This change ensures the publish workflow is run on changes to master by removing the package.json dependency.
This enables us to trigger the workflow without requiring changes to package.json.
Mitigating side-effects:
npm already makes sure existing versions are not re-published. Therefore this change is safe.


Better statistics, downloads per user agent

I see so many downloads are market as unknown, that makes the downloads statistics useless.
Many others are marked as Browser, my package has so many downloads by browser that I wonder why people downloads it with a browser?
Maybe they are crawler, or masked download via proxy, who knows, with this kind of statistics it's impossible to make any kind of analysis.
Since I know you can't interpret any kind of user agent, I suggest to provide a detailed report with download numbers per user agent, so I can parse and analyze them on my own.
It's ok also to have an API or a raw file with a download link if possible.

@OrbintSoft , agree that seeing "unknown" as client is a poor experience. We will analyze which user agents are not parsed correctly and add them to the list (the work is tracked here).
Regarding "Browser" client, this is usually crawlers or user clicking on "Download package" link in on the package details page.

I guess most they crawlers, I noticed soon after every release I get 8 downloads marked as browsers, that happens regularly on both stable and preview versions.
That makes me think that it's something automatic like crawler, true download via browser increase slowly with the time and in any case I expect less downloads by browser than via nuget client.
That makes the statistics useless, I can't even compare downloads between stable and previews releases, because most downloads are made by crawlers.
I guess also most of unknown are crawlers, because numbers are too high too be an unknown packaging manager tool.
I developed a good library for user agent parsing, able to recognize most of crawlers, bot, spiders, browsers, etc... so if it is possible I prefer to have the possibility to download raw data to make my own statistics.

@OrbintSoft , I will add this ask to our backlog. Improving our statistics is definitely important, but due to priority of other work, will not be picked up soon.


[Feature Request] Purely functional loss objectives

Motivation
1. Consistent style for torch.nn.modules.loss.*Loss
In torch.nn.modules.loss, there are many *Loss subclassing nn.Module. The Loss.__init__() does not takes other nn.Module's as arguments. And method Loss.forward() method is purely functional and directly calls nn.functional.*_loss.
I think the motivation for using torch.nn.modules.loss.*Loss is compositing networks by nn.Sequential(...).
2. More straightforward implementation for functional style algorithms, such as meta-RL algorithms
In many meta-RL algorithms, the policy is trained with meta-parameters that may not register to the LossModule.
Case.1 MGRL: Register leaf meta-parameters as buffers in the loss module
For Meta-Gradient Reinforcement Learning (MGRL) https://arxiv.org/abs/1805.09801, it takes the discount factor gamma as the meta-parameter cross RL updates.
Use PPO for example:
import torch
import torch.nn as nn

from torchrl.objectives import PPOLoss

import torchopt

### Setup ###
meta_param = nn.Parameter(torch.tensor(0.95))
loss_module = PPOLoss(
    actor, critic, ...,
    gamma=None,  # whatever value
)
loss_module.regester_buffer('gamma', meta_param)  # register gamma as buffer

### Optimizers ###
inner_optim = torchopt.MetaAdam(loss_module)
outer_optim = torchopt.Adam([meta_param])

### Inner update (update network parameters) ####
inner_loss1 = loss_module(tensordict)
inner_optim.step(inner_loss1['loss_objective'])  # inner update 1: param(0) -> param(1)

...

inner_lossN = loss_module(tensordict)
inner_optim.step(inner_lossN['loss_objective'])  # inner update N: param(N - 1) -> param(N)

### Outer update (update meta-parameters (gamma)) ###
outer_loss = loss_module(tensordict)  # sampled by param(N)
outer_optim.zero_grad()
outer_loss.backward()
outer_optim.step()

See https://github.com/metaopt/TorchOpt#torchopt-as-differentiable-optimizer-for-meta-learning for figures.

we need to register our meta-parameter gamma in the buffer of the loss module instead of full control of the parameters by the user.
For integration with functorch, register the meta-parameter as module buffer works freely.
meta_param = nn.Parameter(torch.tensor(0.95))
loss_module = PPOLoss(
    actor, critic, ...,
    gamma=None,  # whatever value
)
loss_module.regester_buffer('gamma', meta_param)  # register gamma as buffer

# Make functional
fmodel, params, buffers = functorch.make_functional_with_buffers(loss_module)

Case.2 LPG: Register non-leaf meta-parameters as buffers in the loss module on every outer update
For Learning Policy Gradient (LPG) https://arxiv.org/abs/1805.09801, it takes the LSTM network as the meta-parameter.
Different from MGRL, on each update, the meta-network output is not a leaf tensor anymore. Then we need to register these output again and again before each call of loss_module.forward. This makes
fmodel, params, buffers = functorch.make_functional_with_buffers(loss_module)

not working.
cc @Benjamin-eecs @waterhorse1
Solution
A clear and concise description of what you want to happen.
Split the forward method in the loss module into a separate pure function, i.e., a state less function does not have any parameters. The model parameters should be organized by other modules. The loss function only takes a tensordict as input, and add a new key "loss_objective" into the tensordict. All tensor inputs (e.g. value = self.critic(...)) should be calculated before calling the loss function, because the loss function is purely functional, i.e., does not host parameters (e.g., actor.parameters(), critic.parameters()).
Here is a prototype example:
def ppo_loss(tensordict: TensorDictBase, dist_class: Distribution, **kwargs) -> TensorDictBase:
    tensordict = tensordict.clone()
    gamma = tensordict.get('gamma', kwargs.get('gamma'))  # In MGRL, the `gamma` parameter can be a tensor rather than a Python scalar
    critic_coeff = tensordict.get('critic_coeff', kwargs.get('critic_coeff'))
    dist = dist_class(tensordict.get('actor_output'))
    ...

For backward compatibility, refactor the PPOLoss module as:
class PPOLoss(LossModule):
    actor: nn.Module
    critic: nn.Module
    gamma: float
    entropy_coef: float
    critic_coef: float
    advantage_module: nn.Module

    def __init__(self, ...):
        ...

        self.floss_kwargs = dict(
            gamma=self.gamma,
            entropy_coef=self.entropy_coef,
            critic_coef=self.critic_coef,
        )

    def forward(self, tensordict: TensorDictBase) -> TensorDictBase:
        # Prepare all inputs for loss function
        if self.advantage_module is not None:
            tensordict = self.advantage_module(
                tensordict,
            )
        tensordict = tensordict.clone()
        ...

        # Call purely functional version of loss function
        return ppo_loss(
            tensordict,
            dist_class,
            **self.floss_kwargs
        )

Alternatives
A clear and concise description of any alternative solutions or features you've considered.
Copy and paste the loss module source code, then do specific customizations.
Additional context
Add any other context or screenshots about the feature request here.
Checklist

[X] I have checked that there is no similar issue in the repo (required)

Thans for this detailed description @XuehaiPan
Can you elaborate on this?

Case.2 LPG: Register non-leaf meta-parameters as buffers in the loss module on every outer update
For Learning Policy Gradient (LPG) https://arxiv.org/abs/1805.09801, it takes the LSTM network as the meta-parameter.
Different from MGRL, on each update, the meta-network output is not a leaf tensor anymore. Then we need to register these output again and again before each call of loss_module.forward. This makes
fmodel, params, buffers = functorch.make_functional_with_buffers(loss_module)

not working.

As far as I understand it, the parameters of the LSTM (if they're part of the model) will end up in params when calling functorch.make_functional_with_buffers and they won't need to be leaf tensors anymore.
But I guess I misunderstood your point.

Thanks for the answer

We need to maintain indices for this and pass the parameters carefully.
If I understand well what bothers you with the current API is that with functorch as it is not, you are working with a list of parameters/buffers which makes it hard to assign them to a particular module, is that right?

What if we were storing the params in a dictionary instead (or better: a TensorDict :D )?
@zou3519 mentioned there that some users were asking for this. I'm happy to sketch a solution like this if that would unblock you (and/or implement the solution you were suggesting, even temporarily while waiting for a functorch fix if we opt for it).

If I understand well what bothers you with the current API is that with functorch as it is not, you are working with a list of parameters/buffers which makes it hard to assign them to a particular module, is that right?

Yes, that's right.

and/or implement the solution you were suggesting

Thanks for this. The new implementation would resolve both points 1 and 2 in https://github.com/facebookresearch/rl/issues/338#issue-1328464480.


CI: Add Ruby 3.0 to the build matrix

This PR adds Ruby 3.0 to the build matrix in CI.
Got to 🟢 !

Thanks!

Thanks!


reduce warnings

I reduce warnings in the GCEED part that related to variables which are not used in subroutines. I checked that it runs successfully for the ARTED part and the GCEED part.
This PR may cause a conflict. In that case, I fix it.

I forgot to say one thing. Today is holiday, so there is no problem if this PR is not checked today.

Thank you @mn2007. I will check later (by Monday?)...

Thank you for developement, @mn2007.
I grasped this PR removes the useless variables from some GCEED-related subroutines (calc_occupation,copyVlocal,read_copy_pot,rmmdiis,setRps_ABINIT_PBE,setRps_fhi,setRps_fhi,setmg,setng). This modification does not have significant reaction to the ARTED calculation and confirm this modification is available to merge to develop branch.