Refactor: ChangeType in TextChange

To address #322, I reimplemented the solution in #402 by using more readable code via ChangeType, which was through lazy evaluation initially. This caused regressions, as discovered in #472, and was later evaluated immediately in #470 to help find these regressions. Besides simplifying the logic, I used ChangeType because I thought developers would be able to use it in their code to quickly determine what kind of change the change was. By doing this, I realize now that I was solving the first problem and indirectly trying to solve another problem (adding convenience methods to TextChange that allows one to quickly determine what kind of change it was).
And that is the issue: my solution to the second problem was not fully thought out because it was not the issue I was trying to solve. I didn't realize it then (but do realize it now because of the regressions, both known and yet unknown, that have appeared since 0.7-M3) that a RichTextChange can be a number of different changes (insertion, deletion, replacement, text style change, or a paragraph style change)--change tyes which ChangeType does not cover.
So I see two options for addressing this bug:

Refactor the code to always create a TextChange that can get exactly what kind of change type it is
Change the way the code knows that a TextChange is an insertion or deletion type of change and thus whether to merge it with another TextChange.

I'm against the former because it will be more invasive to the current API and using a TextChange does not always require knowing what kind of change it is. For example, sometimes one just needs to know how much to adjust the caret based on how much content was removed and inserted.
I think the latter is best because it will immediately address all possible (known and currently unknown) regressions related to #458.
To still maintain code readability, I don't think we should just implement methods like TextChange#isInsertion and TextChange#isDeletion. Rather, I think ChangeType (which I think needs to be better named since it doesn't address all the changes a TextChange could be and since it pollutes the namespace that developers might want to use later in their own code) should be as follows:
// Enum needs a better name!
public class ChangeType {
    INSERTION,
    DELETION,
    NEITHER // indicates it is neither an insertion/deletion

And then the TextChange#mergeWith code would still read:
// this is either a insertion/deletion
if (this.getType() != ChangeType.NEITHER
    // the two types match: both are insertion or both are deletion
    && this.getType() == that.getType()) {
    // merge code
}

Unfortunately, I can't think of a better name to address the namespace pollution issue I raised above. Does anyone else have a better name?
However, #472 did show that sometimes RTFX is inefficient because it tries to make an empty replacement. I wonder if there are others areas that could be further optimized so that replace always replaces something and ignores calls that ultimately do nothing. For example, ignoring "style change" calls that don't actually restyle the content because that content already has that style.

Does anyone else have a better name?

NONTEXTUAL? FORMATTING?

Thanks for the ideas. However, neither makes much sense to me at first glance. And if one needs to explain it, that's also problematic.
As I thought about this today, I thought the nature of the data is the type of change stored. However, that data is used is to determine whether a merge should occur. So I thought, why not call it MergeType?

@kuc Before merging 487, I'd be curious to hear your thoughts on the naming.

@JordanMartinez OK, I will look on it in a free moment...

@TomasMikula #487 should resolve any known and unknown regressions introduced by #458. Could you make a new milestone release?

The choice that only two insertions or only two deletions will be merged is somewhat arbitrary.
(The previous implementation was not arbitrary in the sense that it was an extremal choice: it merged whenever possible.)
We should avoid making arbitrary choices, at least in the core.
I understand that this is in an attempt to provide more reasonable default behavior. This should be possible to do on top of the universal core, though. Specifically, limiting merges by type of change can be implemented in the function passed to undo manager constructor (it doesn't have to just directly invoke PlainTextChange::mergeWith). On the other hand, users will not be able to reuse the new PlainTextChange::mergeWith if they do want to merge an insertion followed by a deletion.
Aside, I'm skeptical that MergeType will be of much use to the user.

The choice that only two insertions or only two deletions will be merged is somewhat arbitrary.
(The previous implementation was not arbitrary in the sense that it was an extremal choice: it merged whenever possible.)
We should avoid making arbitrary choices, at least in the core.
I understand that this is in an attempt to provide more reasonable default behavior. This should be possible to do on top of the universal core, though. Specifically, limiting merges by type of change can be implemented in the function passed to undo manager constructor (it doesn't have to just directly invoke PlainTextChange::mergeWith). On the other hand, users will not be able to reuse the new PlainTextChange::mergeWith if they do want to merge an insertion followed by a deletion.

In other words, #322 is (in some ways) a symptom of the developer not having complete control over how the undo/redo works. Thus, the approach of #402 / #458 is incorrect and something like #333 would need to be implemented (meaning, giving the developer more control as to how undo/redo works).

Aside, I'm skeptical that MergeType will be of much use to the user.

It wasn't supposed to be of much use, just provide the bare minimum functionality to fix #322 and fix any regressions introduced by #458 so that a milestone release could be made that has all the new functionality since 0.7-M3 without any of the regressions of #458. As stated above in my opening comment, we could make a TextChange store exactly what kind of change it is (option 1), but that would require more work. Since a regression-free milestone release was my goal, I didn't do it. However, I do think it would be worth implementing in a later release.

Specifically, limiting merges by type of change can be implemented in the function passed to undo manager constructor (it doesn't have to just directly invoke PlainTextChange::mergeWith).

True, but this was private and in StyledTextAreaModel until #463. To expose it now, we just need to make it public and final. However, that is inefficient as the constructor will construct a default UndoManager using TextChange::changeWith, which will be overridden by the updated one via setUndoManager(UndoManagerFactory). An UndoManager cannot be created before the area is created (as #333 suggests) now that ESD is the model. So, it makes more sense to include a BiFunction parameter in the constructor that is used as the mergeWith method when the constructor calls create[Plain/Rich]UndoManger().

On the other hand, users will not be able to reuse the new PlainTextChange::mergeWith if they do want to merge an insertion followed by a deletion.

But if they do use the original TextChange::mergeWith, that will still cause #322 at some point. I'm guessing you're suggesting that we update the original version of that method to stop that from happening?

#322 has to be fixed independently of what merging strategy is implemented. We shouldn't get an exception with any strategy.
I see a couple of ways to fix #322.
Solution A
Notice that with the current arguments passed to UndoManagerFactory#create
    <C> UndoManager create(
            EventStream<C> changeStream,
            Function<? super C, ? extends C> invert,
            Consumer<C> apply,
            BiFunction<C, C, Optional<C>> merge);

the UndoManager has no way of knowing whether the result of merge is an empty change. When merge results in an empty change, the UndoManager has no other choice than to store it and, when asked to undo, apply the inverse of that change (which will be empty as well). It then expects this (inverted) empty change to arrive in changeStream.
For RichTextFX to work properly with this behavior of UndoFX, the only possibility is to accept edits which are no-ops and emit corresponding empty changes.
This solution provides the most guarantees to clients (such as the UndoManager): every edit will result in a change event.
Solution B (requires UndoFX changes)
Another option is to give the UndoManager a way to know whether merge resulted in an empty change. This could be done for example by adding an extra parameter Predicate<C> isEmpty to the factory method create above. Undo manager would then test changes for emptiness and automatically discard empty changes without trying to apply them.

Note also that the two solution can coexist, we could both.

I would also like to mention that the set of changes resembles a group. (We don't quite have a group, because not any two changes can be merged.) What this means is that

merge is associative, i.e. for any changes a, b, c:merge(a, merge(b, c)) = merge(merge(a, b), c)

merging any change a with an empty change e leaves a unaffected:merge(a, e) = a
merge(e, a) = a

every change has an inverse:merge(a, invert(a)) = e
merge(invert(a), a) = e

where e is the empty change.

These are useful properties to keep in mind when implementing merge.

For RichTextFX to work properly with this behavior of UndoFX, the only possibility is to accept edits which are no-ops and emit corresponding empty changes.
This solution provides the most guarantees to clients (such as the UndoManager): every edit will result in a change event.

While I get the idea behind Solution A, accepting and emitting empty changes seems silly; an empty change, by its definition, isn't a change at all, just wasted memory/time. Isn't it just better to return immediately on calls of replace(n, n, /* empty document */) where 0 <= n <= length()? There might be good reasons to do a no-op replace call in a developer's project, but I can't imagine such a scenario.
On another hand, how long does it take for a no-op replace call to return? On very large documents, could this be a performance issue? (I doubt it would be too much of a hit because ReactFX's efficient FingerTree lies behind the implementation.)
At the very least, Solution B should be implemented because this issue will arise in any project that uses UndoFX (whether that be RTFX or something else).

When someone's calling replace(n, n, /* empty document */), it is likely because they didn't care to check whether the document is empty (it will not be empty in general, and they don't care for the few cases when it is). If they are going to do N replace operations, some of which potentially empty, they typically have to "allocate budget" for N full replace operations anyway.
But yeah, to give us more flexibility, I would leave it undefined whether empty changes will be emitted. Then users cannot count on empty changes being emitted, neither can they count on any emitted change being non-empty.

At the very least, Solution B should be implemented because this issue will arise in any project that uses UndoFX (whether that be RTFX or something else).

Not if there is a notion of an empty change and empty changes are being silenced.
But I agree it should be implemented anyway.

When someone's calling replace(n, n, /* empty document */), it is likely because they didn't care to check whether the document is empty (it will not be empty in general, and they don't care for the few cases when it is).

Right, which is why the replace method can check if the document is empty and return immediately if it is. The user does not have to worry about that.

If they are going to do N replace operations, some of which potentially empty, they typically have to "allocate budget" for N full replace operations anyway.

"Allocate budget?" Not sure what you mean by that. Regardless, doesn't the "return immediately on empty document" approach deal with anyway?

Not if there is a notion of an empty change and empty changes are being silenced.

What do you mean by "empty changes are being silenced?" You seem to be implying that there are projects that might have an empty change (the first half of your statement). But it's the second half that leaves me puzzled.

When someone's calling replace(n, n, /* empty document */), it is likely
because they didn't care to check whether the document is empty (it will
not be empty in general, and they don't care for the few cases when it is).
Right, which is why the replace method can check if the document is empty
and return immediately if it is. The user does not have to worry about that.
If they are going to do N replace operations, some of which potentially
empty, they typically have to "allocate budget" for N full replace
operations anyway.
"Allocate budget?" Not sure what you mean by that. Regardless, doesn't the
"return immediately on empty document" approach deal with anyway?
CPU budget. That they have to account for the worst case: none of the
operations being no-op.
Not if there is a notion of an empty change and empty changes are being
silenced.
What do you mean by "empty changes are being silenced?" You seem to be
implying that there are projects that might have an empty change (the first
half of your statement). But it's the second half that leaves me puzzled.
It should have been "Only if" instead of "Not if". By being silenced I mean
not emitted.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/TomasMikula/RichTextFX/issues/486#issuecomment-293769962,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAadH9XeItv25GkEbpTJyoXYTQtaZdbcks5rvZeEgaJpZM4M5OK_
.

CPU budget. That they have to account for the worst case: none of the operations being no-op.

Oh ok.

It should have been "Only if" instead of "Not if". By being silenced I mean not emitted.

Ok, I thought that's what you meant. Then, couldn't one return false on the UndoManager's isEmpty (or the perhaps better-named isNoOp) predicate if they didn't want empty changes being silenced:
Predicate<Change> isNoOpChange = c -> false;

Ah, my email reply didn't get formatted well. But you figured out what was quoted and what was new, anyway :)

Then, couldn't one return false on the UndoManager's isEmpty (or the perhaps better-named isNoOp) predicate if they didn't want empty changes being silenced:
Predicate<Change> isNoOpChange = c -> false;

Yes, but the discussion was about the current UndoFX. You said

this issue will arise in any project that uses UndoFX

and I said (well, meant to say) that only if that project has a notion of empty changes and silences them.

Ah, my email reply didn't get formatted well. But you figured out what was quoted and what was new, anyway :)

Yeah... I was a bit confused about the formatting at first :smile:

Yes, but the discussion was about the current UndoFX.... and I said (well, meant to say) that only if that project has a notion of empty changes and silences them.

Ah, yeah, I was talking about the updated version of UndoFX

I don't have the time right now to re-read through this issue, but right now replace returns immediately if the replacement is an empty document. I'm simply making note of this so that, if that is an issue, I don't forget to remind everyone of it so we can fix it later.

Released UndoFX 1.3.0 with the required feature.

Referring to your comment, I still think Solution A is silly, but I agree that its necessary in case others want to implement their own merge method. So, we'll need to remove the return statement that gets called if an empty document is the parameter in replace. After that, we can use the new "ignore identity changes" feature.
However, one question arises. In a RichTextChange, what is the identity change since style changes also count as a change? We can't use initialParagraphStyle or initialTextStyle because those can change after initialization.
Before I forget, I should reopen this issue.

I still think Solution A is silly, but I agree that its necessary in case others want to implement their own merge method.

Not if they also provide a compatible isIdentity function?

However, one question arises. In a RichTextChange, what is the identity change since style changes also count as a change?

Is there a problem with defining identity change as a change where the inserted document equals the removed document?

Is there a problem with defining identity change as a change where the inserted document equals the removed document?

Ah, that would be it.

Now that #495 is merged, should we now revert back to the previous merge method? Or is that the new issue to tackle?

At the very least, I think at the very least MergeType doesn't need to exist as a type.
I realize I had a bad memory of the previous implementation when I said

The previous implementation was not arbitrary in the sense that it was an extremal choice: it merged whenever possible.

It didn't merge whenever possible, but only when the next change continued at the place where the previous one finished. I think this was a somewhat reasonable default, and did not suffer from #493.
As a matter of principle, I still think providing the most extreme choice is a good idea, but doesn't need to be addressed now.
In the future, we might want to support non-contiguous changes, so don't spend too much time on something that might be replaced anyway.

I think that at the very least MergeType doesn't need to exist as a type.

Agreed.

I think this was a somewhat reasonable default, and did not suffer from #493. As a matter of principle, I still think providing the most extreme choice is a good idea, but doesn't need to be addressed now.

So, what I'm reading is we should revert back to the original merge method now and figure out a more extreme choice later on. Those who don't want to use that method can implement their own.

In the future, we might want to support non-contiguous changes

How would that work?

How: The representation of a change would basically be a list of things we today call a change.
Why: Ability to represent global find/replace as a single undo item.

How: The representation of a change would basically be a list of things we today call a change.

I assumed such would be the case when #222 gets implemented (List<TextChange<S>>), so that one could rename a method. But the context of the merge method and the way you phrased it sounded like you were going to do some voodoo magic or something :laughing:

Closed by #496


Added Getting Started

closes #1

We love contribution, but we suggest you try and submit more meaningful PRs by exploring any of our open issues. If you have any problem or need help, please join our Discord community: https://appwrite.io/discord and use the #maintainers channel.


Move metrics reporting into its own event handler.

Proposed Changes
This breaks the "state keeping" of the KnativeServing reconciler out of the reconciler and pushes it into a stateless event-handler format. Also moves the stats reporter code into a KnativeServing specific package.
Release Note

NONE

/assign @aliok @houshengbo

/test pull-knative-sandbox-operator-upgrade-tests

/lgtm

/approve


Add support for python 3.6

This change adds support for building pystache for python 3.6.

Hi! This is the friendly automated conda-forge-linting service.
I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.
Here's what I've got...
For recipe:

The recipe license should not include the word "License".

Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

Might be worth giving noarch: python a try. This will avoid having to rebuild for new Pythons.

I think this PR is now properly ready for review. Thanks for your patience with all the incremental changes, this is my first time working with conda smithy.

Also would get rid of bld.bat in the recipe. That's a really old legacy workaround from some ancient conda-build 1.x that no longer exists.

Hi @jakirkham, would you be able to review/merge this PR?

LGTM. Let's see what @kwilcox thinks.

I could have done with a rebase for a cleaner commit history... Next time!

Thanks for the review @kwilcox, will definitely aim for a clean rebase next time, sorry about that!

@kwilcox  It looks like the CircleCI tests failed on master after the PR merge, thought they were fine on this branch. Will this prevent the package ending up on conda-forge? Do these tests need to be retriggered somehow?

The SSH key used to checkout the code on CircleCI expired. We have refreshed it and it built through. Please let us know if there are other issues.


Allow HEROKU_CONNECT_SCHEMA to be set from the settings

This setting was hardcoded, but we need it to be flexible.

@codingjoe are there any tests I should consider changing for this one?

@codingjoe thanks, I did update a docstring.
Offtopic: I did push a commit, saw LambdaLint complaining, then amended some fixes. And now it is waiting for status to be reported. 🤔

@amureki In general you wrap right before the first word that breaches the 80 limit. I didn't know how to explain myself better. I did add a commit to explain what I meant. Do you

@codingjoe sorry, it is just not hard-pointing in my editor, as well as we don't have any linter here in this repo installed. That's why I was checking other lines (that are also not strictly following the 80 chars rule).
We should consider adding the linter then and fixing all the places. :)

Released in 0.18.0

@amureki I would certainly make it easier for everyone.


ip_range_to_cidr converts all ranges to /1 CIDR

This problem was found while looking for a workaround for issue #1, as I just wanted to one-time copy the list to my npm config.
The function "ip_range_to_cidr" is transforming any IP range to a 1 bit subnet mask, which is ... kinda bad, for obvious reasons.
some examples:

CSV Input
should be CIDR
generated CIDR
resulting range

<IP_ADDRESS>,<IP_ADDRESS>,CH
<IP_ADDRESS>/23
<IP_ADDRESS>/1
<IP_ADDRESS>-<IP_ADDRESS>

<IP_ADDRESS>,<IP_ADDRESS>,CH
<IP_ADDRESS>/24
<IP_ADDRESS>/1
<IP_ADDRESS>-<IP_ADDRESS>

<IP_ADDRESS>,<IP_ADDRESS>,CH
<IP_ADDRESS>/24(?)
<IP_ADDRESS>/1
<IP_ADDRESS>-<IP_ADDRESS>

Working on a fix atm.

here is my fix:
def ip_range_to_cidr(ip_start, ip_end):
    # calculate all containing subnets
    subnets = [str(ipaddr) for ipaddr in ipaddress.summarize_address_range(ipaddress.IPv4Address(ip_start), ipaddress.IPv4Address(ip_end))]
    # return list of subnet(s)
    return subnets

(I think the function is obsolete now, as the one liner can just sit inside the code)
this function may return a list of subnets now (like in the example above "<IP_ADDRESS>,<IP_ADDRESS>" will not result in a clean subnet, and will therefore end up in multiple clean subnets). therefore we need to loop trough the subnet list:
access_rule_list = [npmclient.Access_Rule_Client(address=range, directive='allow') for row in filtered_entries.itertuples(index=False) for range in ip_range_to_cidr(row.start_ip, row.end_ip)]

hopefully the additional subnets this will not make #1 much worse 😬

hopefully the additional subnets this will not make https://github.com/lavinir/npm-geoaccesslists/issues/1 much worse 😬

well ...
Found 3692 entries for ['CH']
Subnet count: 5508

... sorry. it's an improvement anyway imho.

@bitKrakenCode thank you tor opening this topic as I noticed the same:
This is my sample list:
<IP_ADDRESS>,<IP_ADDRESS>,GB
<IP_ADDRESS>,<IP_ADDRESS>,GB
<IP_ADDRESS>,<IP_ADDRESS>,GB
<IP_ADDRESS>,<IP_ADDRESS>,GB

which resulted in:
allow <IP_ADDRESS>/1
allow <IP_ADDRESS>/1
allow <IP_ADDRESS>/1
allow <IP_ADDRESS>/1
deny all

but a network calculator for the first allow:
<IP_ADDRESS>/1
shows
Address:   <IP_ADDRESS>
HostMin:   <IP_ADDRESS>
HostMax:   <IP_ADDRESS>

So thank you for your workaround code!
/24 netmasks are ok now
However
<IP_ADDRESS>,<IP_ADDRESS>,GB
results in
<IP_ADDRESS>/15
HostMin:   <IP_ADDRESS>
HostMax:   <IP_ADDRESS>

Not much of a big problem, though.
Did you managed to overcome the issue mentioned in #1 ? For a given country, it seems Network Proxy Manager will not be able to create such a big allow list...

@mixpc

which resulted in:
allow <IP_ADDRESS>/1
allow <IP_ADDRESS>/1
allow <IP_ADDRESS>/1
allow <IP_ADDRESS>/1
deny all

Weird, I get this:
    # Access Rules: 52822 total
    allow <IP_ADDRESS>/24;
    allow <IP_ADDRESS>/15;
    allow <IP_ADDRESS>/24;
    allow <IP_ADDRESS>/24;

However
<IP_ADDRESS>,<IP_ADDRESS>,GB
results in
<IP_ADDRESS>/15
HostMin:   <IP_ADDRESS>
HostMax:   <IP_ADDRESS>

"HostMin" / "HostMax" just means, that the range of usable IPs for your clients are <IP_ADDRESS> to <IP_ADDRESS>.
<IP_ADDRESS> is the network address, <IP_ADDRESS> is the broadcast address: you can't assign them to a client in your network. they are still included in <IP_ADDRESS>/15

Did you managed to overcome the issue mentioned in https://github.com/lavinir/npm-geoaccesslists/issues/1 ? For a given country, it seems Network Proxy Manager will not be able to create such a big allow list...

I just tried it for the first time in a while: seems to be working now. but maybe I have some limit set in my script: so not 100% sure about that (but the total count for the access list seems to be fine now).

@lavinir #1 seems to be fine now and this issue could be fixed with my suggested code. you can change it yourself or give me access to the repo to open a PR for it: whatever you prefer. thanks!

@bitKrakenCode thank you for your feedback. Just make it clear, with your workaround I have been able to add IPs to the ACL. However, after running several tests I have noticed that whe ip-list-file file has more than 1,000 lines (on average) add-npm-geofilter.py will output the Error adding access list. (413) from #1 If the file has about 1,000 lines or less, it will be added to the ACL successfully. If the source ip-list-file has more than 1,000 lines (on average) the only solution I have found is split it in chunks and create as many ACLs as chunks, and then add each chunk to a different ACL. And then, yes, nginx proxy manager, will be able to take all those IPs addresses into account. I don't know whether the limit to the approx number of lines comes from the nginx proxy manager api or add-npm-geofilter.py  or some other limitation.

@mixpc strange. I'm pretty sure, I didn't implement a workaround for this on my system and it seems to work with a 50'000+ address list. what version of npm are you using?

Hello,
services:
  nginx-proxy-manager:
   image: jc21/nginx-proxy-manager:latest

v2.11.1
Tried again from scratch. Just interested in ip v4 addresses (shorter list) so 13,972 lines in ip_fr.txt from dbip-country-lite-2024-02.csv
python add-npm-geofilter.py --npm-host <IP_ADDRESS> --npm-port 81 --npm-email<EMAIL_ADDRESS>--npm-password 123 --npm-accesslist-name FR --allowed-countries FR --ip-list-file ip_fr.txt
Found 13972 entries for ['FR']
Getting auth token
Adding access list
adding to ruleid : 9
Error adding access list. (413)

I can try with other countries if needed. Maybe just downgrade nginx-proxy-manager to any given version?

@mixpc ok lol 🙃 I did implement a workaround by inserting the list directly in the sqlite DB. so #1 is probably not fixed for now.
If you want to implement it too:
replace this line (should be line nr 64):
npmclient.add_access_list(args.npm_accesslist_name, token, existing_rule_id, *access_rule_list)
with this code (and adjust the path to the sqlite file):
import sqlite3
import datetime

# Create a connection object
connection = sqlite3.connect("/path/to/nginxproxymanager/database.sqlite")

# Create a cursor object
cursor = connection.cursor()

# Execute a CREATE TABLE statement
cursor.execute(f"DELETE FROM access_list_client WHERE access_list_id = {existing_rule_id}")

for row in access_rule_list:
    # allow <IP_ADDRESS>/22;
    # address='<IP_ADDRESS>/23', directive='allow'
    # format
    # datetime.datetime.now()
    query_value_meta = '{}'
    cursor.execute(f"INSERT INTO access_list_client (created_on, modified_on, access_list_id, address, directive, meta) VALUES ('{datetime.datetime.now()}', '{datetime.datetime.now()}', {existing_rule_id}, '{row.address}', '{row.directive}', '{query_value_meta}')")

print("synced to DB")

# Commit the transaction
connection.commit()

# Close the connection
connection.close()

I will probably add the workaround to #1 too.

@bitKrakenCode Excellent! No need for sorries; glad to provide feedback!
sudo python add-npm-geofilter.py --npm-host <IP_ADDRESS> --npm-port 81 --npm-email<EMAIL_ADDRESS>--npm-password 123 --npm-accesslist-name FR --allowed-countries FR --ip-list-file ip_fr.txt
Found 13972 entries for ['FR']
Getting auth token
Adding access list
synced to DB

Amazing! Thank you @bitKrakenCode 😃

here is my fix:
def ip_range_to_cidr(ip_start, ip_end):
    # calculate all containing subnets
    subnets = [str(ipaddr) for ipaddr in ipaddress.summarize_address_range(ipaddress.IPv4Address(ip_start), ipaddress.IPv4Address(ip_end))]
    # return list of subnet(s)
    return subnets

(I think the function is obsolete now, as the one liner can just sit inside the code)
this function may return a list of subnets now (like in the example above "<IP_ADDRESS>,<IP_ADDRESS>" will not result in a clean subnet, and will therefore end up in multiple clean subnets). therefore we need to loop trough the subnet list:
access_rule_list = [npmclient.Access_Rule_Client(address=range, directive='allow') for row in filtered_entries.itertuples(index=False) for range in ip_range_to_cidr(row.start_ip, row.end_ip)]

hopefully the additional subnets this will not make #1 much worse 😬

Hi. What exactly do I have to do to fix the script? Do I have to replace the existing "def ip_range_to_cidr(ip_start, ip_end)" and "access_rule_list" functions completely with your code?
Thank you.


missing log lines due to terminal escape sequences (when running with --verbose)

Couldn't figure out how to test it automatically but I added a test which will hopefully show the problem on travis

This actually makes sense.
Solution would be to remove the nifty progress display when running in verbose mode.

@akariv thanks, fixed in the cli, updated the test to ensure there are no escape sequences when running with --verbose (and that there are when running without verbose)


DirectoryNotFoundException after updating from 1.3.9 to 1.4.5

After an update to 1.4.5 I'm getting this every few seconds:
DirectoryNotFoundException: Could not find a part of the path " *PathToMyUnityProjectFolder*\Assets\Assets\Plugins\Editor".
System.IO.Enumeration.FileSystemEnumerator`1[TResult].CreateDirectoryHandle (System.String path, System.Boolean ignoreNotFound) (at <d6232873609549b8a045fa15811a5bd3>:0)
System.IO.Enumeration.FileSystemEnumerator`1[TResult]..ctor (System.String directory, System.IO.EnumerationOptions options) (at <d6232873609549b8a045fa15811a5bd3>:0)
System.IO.Enumeration.FileSystemEnumerable`1+DelegateEnumerator[TResult]..ctor (System.IO.Enumeration.FileSystemEnumerable`1[TResult] enumerable) (at <d6232873609549b8a045fa15811a5bd3>:0)
System.IO.Enumeration.FileSystemEnumerable`1[TResult]..ctor (System.String directory, System.IO.Enumeration.FileSystemEnumerable`1+FindTransform[TResult] transform, System.IO.EnumerationOptions options) (at <d6232873609549b8a045fa15811a5bd3>:0)
System.IO.Enumeration.FileSystemEnumerableFactory.UserEntries (System.String directory, System.String expression, System.IO.EnumerationOptions options) (at <d6232873609549b8a045fa15811a5bd3>:0)
System.IO.Directory.InternalEnumeratePaths (System.String path, System.String searchPattern, System.IO.SearchTarget searchTarget, System.IO.EnumerationOptions options) (at <d6232873609549b8a045fa15811a5bd3>:0)
System.IO.Directory.EnumerateFileSystemEntries (System.String path, System.String searchPattern, System.IO.EnumerationOptions enumerationOptions) (at <d6232873609549b8a045fa15811a5bd3>:0)
System.IO.Directory.EnumerateFileSystemEntries (System.String path, System.String searchPattern, System.IO.SearchOption searchOption) (at <d6232873609549b8a045fa15811a5bd3>:0)
DevLocker.VersionControl.WiseSVN.SVNStatusesDatabase.GatherGlobalIgnoresInThread (System.Collections.Generic.List`1[T] foundIgnoredEntries) (at Library/PackageCache/devlocker.versioncontrol.wisesvn@482fb63d59/Editor/SVNStatusesDatabase.cs:383)
DevLocker.VersionControl.WiseSVN.SVNStatusesDatabase.GatherDataInThread () (at Library/PackageCache/devlocker.versioncontrol.wisesvn@482fb63d59/Editor/SVNStatusesDatabase.cs:201)
DevLocker.VersionControl.WiseSVN.Utils.DatabasePersistentSingleton`2[SingletonType,DataType].GatherData () (at Library/PackageCache/devlocker.versioncontrol.wisesvn@482fb63d59/Editor/Utils/DatabasePersistentSingleton.cs:155)
UnityEngine.Debug:LogException(Exception)
DevLocker.VersionControl.WiseSVN.Utils.DatabasePersistentSingleton`2:GatherData() (at Library/PackageCache/devlocker.versioncontrol.wisesvn@482fb63d59/Editor/Utils/DatabasePersistentSingleton.cs:189)
System.Threading.ThreadHelper:ThreadStart()

Any ideas?
P.S.: Thank you for maintaining this very usefull tool!

Can you please open cmd in the root of your Unity project and run this command and post it here (censor any sensitive information):
svn propget svn:global-ignores --depth=infinity -v
Thanks

Looks like you have defined in the svn:global-ignores at root level the following pattern: "Assets\Plugins\Editor" which doesn't seem work in SVN. Both svn:global-ignores and svn:ignore ignore paths with folders that include / or \ (unlike git ignore files).
svn:ignore applies only names (with wildcard) to the direct files and folders of the location the property is applied.
svn:global-ignores works the same, but it applies those rules recursively for every folder.
Remove the entry in question from the root folder and apply the following SVN property on the "Assets\Plugins" folder:
svn:ignore set Editor*

More info:
https://tortoisesvn.net/docs/release/TortoiseSVN_en/tsvn-dug-ignore.html
https://tortoisesvn.net/docs/release/TortoiseSVN_en/tsvn-dug-propertypage.html
Let me know if that solved the issue for you.
I'll add a special error message for such cases.

Nice, yes that indeed fixed it! I had to remove every line in svn:global-ignores that contained the Asset folder, but thats not a problem for now.


[talker_bloc_logger] Since upgrading to versions 4.x.x, not seeing any logs anymore (Flutter 3.19.3)

Describe the bug
I just upgraded the talker_bloc_logger package from version 2.4.0 to latest 4.0.3, and noticed that suddenly I don't see any bloc logs anymore. I also recently upgraded to latest Flutter version 3.19.3, so it could be related.
To Reproduce
Steps to reproduce the behavior:

Install Flutter version 3.19.3
Install talker_bloc_logger version 4.0.3
Start the app, and notice that there are no bloc logs

Expected behavior
I would expect the logs to just show normally
Smartphone (please complete the following information):

Device: iOS Simulator + Android Emulator
OS: iOS/Android

Hello @WieFel !
Can you provide info about your code editor ?

Sure!

Android Studio version info
Android Studio Iguana | 2023.2.1
Build #AI-232.10227.8.2321.11479570, built on February 21, 2024
Runtime version: 17.0.9+0-17.0.9b1087.7-11185874 aarch64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.
macOS 14.3.1
GC: G1 Young Generation, G1 Old Generation
Memory: 2048M
Cores: 12
Metal Rendering is ON
Registry:
    debugger.new.tool.window.layout=true
    ide.experimental.ui=true

Non-Bundled Plugins:
    Dart (232.10305)
    com.bloc.intellij_generator_plugin (3.4.0)
    com.github.copilot (<IP_ADDRESS>48)
    io.flutter (78.2.2)

Flutter doctor
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.19.3, on macOS 14.3.1 23D60 darwin-arm64, locale en-IT)
[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
[✓] Xcode - develop for iOS and macOS (Xcode 15.3)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2023.2)
[✓] VS Code (version 1.85.1)
[✓] Connected device (3 available)
[✓] Network resources

• No issues found!

Same behavior with talker_dio_logger

This also happens in talker_dio_logger. Having:
 _dio.interceptors.add(
        TalkerDioLogger(
          settings: const TalkerDioLoggerSettings(
            printRequestHeaders: true,
          ),
        ),
      );

any update?

still not resolved?

Hello everyone!
To prevent issues with log output, you need to pass "talker" as a parameter to all third-party libraries in the "talker" ecosystem.
You can use the following code for the TalkerDioLogger:
final talker = TalkerFlutter.init();
    _dio = Dio();
    _dio.interceptors.add(
      TalkerDioLogger(
        talker: talker,
      ),
    );

Why is this happening?
❗ Log messages integrity

And now I move back default Talker output method for mobile platforms.
Please check how TalkerDioLogger works in your applications with version 4.1.4.
  talker_flutter: ^4.1.4
  talker_dio_logger: ^4.1.4
  talker_bloc_logger: ^4.1.4


make JaxApplet independent with ARTIQ master

Currently all applets try to connect to the ARTIQ master when they starts. This is not necessary for nearly all applets except log, scheduler, and experiment selector. If the artiq server implements all capability of the ARTIQ master (scheduling and logging), the applets mentioned above do not need to connect to the ARTIQ master either.

Actually this is highly nontrivial. When opening applets in a dashboard, the applets dock that starts the applets needs to open connections with the applets (see artiq.gui.applets._AppletDock). Otherwise embedding does not work correctly (widgets are shown as separate windows).


extract_fa -ha broken

Steps to reproduce the issue
Extract half_adder.zip
Using latest build gives error, before it was returning OKAY
yosys -p 'read_verilog top.v ; synth ; extract_fa -ha -v ; write_verilog synth.v'

iverilog testbench.v synth.v  /usr/local/share/yosys/simlib.v

./a.out  should give OKAY

is a duplicate of https://github.com/YosysHQ/yosys/issues/1284 and other one have more info


[cross_file] file.name always returns empty string on desktop if file is created using XFile.fromData

XFile.fromData implementation of io platform is missing name information assignment.
io.dart

Whereas web implementation is correct.
html.dart

Steps to Reproduce
  final file = XFile.fromData(
    bytes,
    length: bytes.length,
    name: 'Hello.png', // Name of the file
    lastModified: DateTime.now(),
  );

  print(file.name); // Prints empty string on web and prints Hello.png on web.

Expected results: file.name should not be empty on io platforms (e.g. desktop)
Actual results: file.name is empty on io platforms (e.g. desktop)

Duplicate of https://github.com/flutter/flutter/issues/87808
See also https://github.com/flutter/flutter/issues/87812


WIP support for FP16/BF16 in train_gpt2.cu (compiles, not correct yet)

Now finished and reasonably happy with it!
1.86x performance on my RTX 4090:

FP32: ~80ms
BF16: ~43ms (with FP32 layernorm params)

This allows the same train_gpt2.cu to work as full FP32, full BF16, full FP16, or full (BF/FP)16 + FP32 layernorm simply by changing the define at the top of the file. Also included stochastic rounding for the Adam kernel (but nothing else at this point, possibly worth adding to gradients in general when we move to FP8?)
To simplify the logic compared to the first version of the PR, all activation tensors are now always "floatX", we cannot mix-and-match. However, because atomicAdd on 16-bit values in some of the backwards kernels are HORRIBLY slow (10x slower or worse), and because this kind of flexibility seems useful in general for layernorm accuracy, layernorm is kept at FP32 by defining "floatN" as "float".
I reduced the amount of code duplication by using very lightweight templates for the kernel types. It's still a BIG change though, unfortunately I don't think there's any way around that!

It is trivial to use the exact same code with everything in FP32, at the top of train_gpt2.cu simply replace this:
typedef __nv_bfloat16 floatX;
#define CUBLAS_LOWP CUDA_R_16BF
with this:
typedef float floatX;
#define CUBLAS_LOWP CUDA_R_32F

This is now able to train in BF16 for many layers and kinda-sorta works for test_gpt2.cu, as the loss converges much slower than FP32 for now (need to debug how to improve that afterwards):
LOSS MISMATCH AT STEP 1: 4.598247 4.059707
LOSS MISMATCH AT STEP 2: 4.152971 3.375123
LOSS MISMATCH AT STEP 3: 3.828835 2.800783
LOSS MISMATCH AT STEP 4: 3.538793 2.315382
LOSS MISMATCH AT STEP 5: 3.260888 1.849029
LOSS MISMATCH AT STEP 6: 3.000814 1.394656
LOSS MISMATCH AT STEP 7: 2.768756 0.999147
LOSS MISMATCH AT STEP 8: 2.557551 0.624080
LOSS MISMATCH AT STEP 9: 2.352901 0.376511
It does eventually converge:
step 99: loss 0.001294 (took 8.712021 ms)

Debugged the BF16 convergence issue and fixed it by adding stochastic rounding support. Simplified code by making all activations the same type (but params can still be different types partly due to severe perf issues for atomicAdd otherwise).
The PR is now in a good state in my opinion where it's worth thinking about what it would take to integrate it.

merging this. we'll iterate in master.


Not working on ARM architectue

Could you build the docker image also for ARM architecture? Currently it doesn't run on my Raspberry Pi.
:-(
kubectl -n clustersecret logs clustersecret-5f98958756-mxs8b
standard_init_linux.go:228: exec user process caused: exec format error

@jeromba6 can you test it?
https://github.com/zakkg3/ClusterSecret/#step-by-step
git pull
kubectl apply -f yaml/arm32v7/02_deployment.yam

are u using k3s ?


Support CurrentUserGuild features

A list of features are in the CurrentUserGuild type.
Documentation: https://github.com/discord/discord-api-docs/commit/8ec82a3c4345d997969002238d961c3264dd1ef9

Never mind, we already support this apparently. It's just out of order.


Revert logo top left

they forgot to change it now they changed it pls fix this it hurts my eyes i wanna die

/* watermark */
@font-face {
  font-family: 'Discord';
  font-weight: 700;
  src: url("https://raw.githubusercontent.com/powercord-org/powercord-backend/my-awesome-branch/web/src/assets/discord-font.otf") format("opentype");
}
.wordmark-2iDDfm:after {
  content: "Discord";
  font-size: 15px;
  position: absolute;
  width: 4000px;
  top: 5px;
  padding-left: 0px;
  font-family: 'Discord';
}
.wordmark-2iDDfm > svg {
  width: 0px;
}

Are you not on the stable build? It looks fine on stable
https://i.imgur.com/It5oiln.png

Are you not on the stable build? It looks fine on stable
https://i.imgur.com/It5oiln.png

its fixed now it wasn't at the time of the post


Remove deprecated children of

Creating a new pretext project off of this template results in a warning out of the box:
* PTX:DEPRECATE: (2023-08-08) a "website" element with "address" and "name" children has changed.  Continue to use the "website" element as before, but replace the "address" and "name" children with a single "url" element, which is more flexible and reliable.  We will try to honor your intent, but you may prefer your own adjustments. (1 time)
    *              located within: "frontmatter" (xml:id)
    * --------------

This removes the deprecated <address> and <name> from the template and replaces them with <url> as suggested.

Thanks for pointing this out.  I guess this is a recent change (and needs to be updated in the Schema of pretext).
I'll merge this request here, but a long-term fix requires an update to the CLI, since this repository is updated with a pretext new every time the CLI has a new release.


Recommended Branch protection rules for automatically merging PR raised by pull

Can we please update recommended Branch protection rules for automatically merging PR raised by pull in the documentation.

Branch protection rules differ by repo, the most common use-case is to not have a branch protection rule on master so pull can automatically merge the PR.
Could you elaborate what you would like to include in the documentation?

@wei I want to have branch protection rules so that no person can merge into master directly but I want to allow pull to merge PR to master automatically. Is it possible?

I don't think it's possible with just branch protection rules.

Thanks


I am using 3.0 USB but in intel realsense it is showing 2.0

librealsense
2.38.1 RELEASE

OS
Windows

Please provide a description of the problem

Hi @ranjitkathiriya  Could you please first try unplugging the camera, waiting a couple of seconds and then re-inserting it into the USB port with a quick and firm motion.  This should help to eliminate the possibility of whether a slow insertion into the USB port is causing the camera to be mis-detected as being on a USB2 connection.
If that does not make a difference, then please open the RealSense Viewer program, go to the More option at the top of its options side-panel and select Hardware Reset from the More option's menu to see whether resetting the camera whilst it is plugged into the port makes a difference to its USB status.

I had a few days ago similar issues with 2.38.1 (using the SDK which printed an error message to log and also using the realsense viewer gui). It turned out that plugging our cam (T265) directly to the PC yielded recognition as USB3 but inserting a USB3 hub in between led to recognition as USB2. Using Windows10.

Hi @burowvrmagic   I am not able to handle T265-related issues as another member of the RealSense team is responsible for those.  Could you create a new case for your issue using the New Issue button on the front page of the RealSense GitHub forum (linked to below) and include 'T265' in the message title, please?  Thanks!
https://github.com/IntelRealSense/librealsense/issues

Hi @burowvrmagic    Do you require further assistance with this case, please?   Thanks!


Fix link for fs2 in README.md

Codecov Report

Merging #1099 into master will decrease coverage by 0.49%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #1099     +/-   ##
=========================================
- Coverage   81.04%   80.55%   -0.5%
=========================================
  Files          54       54
  Lines        1018     1018
  Branches       59       59
=========================================
- Hits          825      820      -5
- Misses        193      198      +5

Impacted Files
Coverage Δ

core/src/main/scala/io/finch/Input.scala
91.8% <0%> (-8.2%)
:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3d374c...c605073. Read the comment docs.

Thank you @akiomik!


[New game]: Connect Four multiplayer game

🎮 Game Request
Game logic and basic description:
Connect Four is a classic two-player board game where the objective is to connect four of your colored pieces in a row, column, or diagonal before your opponent does.
This project is a simple implementation of the Connect Four game using HTML, CSS, and JavaScript.
Point down the features
Game points:
The game starts with the "red" player. Click on any of the seven columns at the top of the game board to drop a red piece into that column.
The red piece will fall to the lowest available row in the selected column. The "red" player's turn is now over.
The game will automatically switch to the "yellow" player's turn. Click on any of the remaining columns to drop a yellow piece into that column.
The yellow piece will fall to the lowest available row in the selected column. The "yellow" player's turn is now over.
Players take turns dropping their respective colored pieces until one player successfully connects four of their pieces in a row, column, or diagonal.
If a player connects four pieces, an alert will pop up to declare that player as the winner.
If neither player connects four pieces and the board is full, the game will end in a draw.
To play again, simply refresh the web page to reset the game board and start a new game.
Select program in which you are contributing
GSSoC23
Code of Conduct

[X] I follow CONTRIBUTING GUIDELINE of this project.

Hey @Swapnil-2503 !
Thank you for raising an issue 💗
You can self assign the issue by commenting /assign in comment 😀
Make sure you follow CODE OF CONDUCT and CONTRIBUTING GUIDELINES 🚀
Don’t Forget to ⭐ our GameZone🎮
Make sure you join our Discord🕹

Hey @Swapnil-2503 ! Thank you so much for your raising the issue💗
It’s all yours, you can come anytime again and make some contributions! 🚀
Alone, we can do little, but together we can do so much! 😇


oscdimg does not support long path

Hello,
First of all thanks for the great tool, it is really helpful !
I have however stumbled upon a limitation with one of the ADK executables, oscdimg, that does not seem to support long path (> 260 characters). As an easy fix, I've tried mapping the Apps folder as a drive using the command subst. But well, what should have happened has happened and I'm still stuck with another path that is too long.
Too bad as oscdimg is a nice one-liner but I'll try with https://github.com/TheDotSource/New-ISOFile and keep you posted here.

Can you try setting LongPath support?
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\FileSystem
Set LongPathsEnabled = 1 (REG_DWORD)
Let me know if that works (preferably without a reboot). If it works without a reboot, we could make the change in the script to enable it and then set it back when finished. I've actually run into issues with long paths and driver downloads (can't remember the exact problem, think BITS didn't like the length of the path or something). So it's possible this fix could help a couple of things.

Seems like I forgot to mention that I did try that without success. The
register and the group policy (though they might be the same thing)
On Wed, Sep 18, 2024, 7:18 PM rbalsleyMSFT @.***> wrote:

Can you try setting LongPath support?
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\FileSystem
Set LongPathsEnabled = 1 (REG_DWORD)
Let me know if that works (preferably without a reboot). If it works
without a reboot, we could make the change in the script to enable it and
then set it back when finished. I've actually run into issues with long
paths and driver downloads (can't remember the exact problem, think BITS
didn't like the length of the path or something). So it's possible this fix
could help a couple of things.
—
Reply to this email directly, view it on GitHub
https://github.com/rbalsleyMSFT/FFU/issues/80#issuecomment-2359018197,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABDS626V4S23SSGH6RC5XHDZXGYXDAVCNFSM6AAAAABONFH4DGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJZGAYTQMJZG4
.
You are receiving this because you authored the thread.Message ID:
@.***>

Did you try a reboot?
From: Laurent Louf @.>
Sent: Wednesday, September 18, 2024 11:27 AM
To: rbalsleyMSFT/FFU @.>
Cc: Richard Balsley @.>; Comment @.>
Subject: Re: [rbalsleyMSFT/FFU] oscdimg does not support long path (Issue #80)
Seems like I forgot to mention that I did try that without success. The
register and the group policy (though they might be the same thing)
On Wed, Sep 18, 2024, 7:18 PM rbalsleyMSFT<EMAIL_ADDRESS>wrote:

Can you try setting LongPath support?
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\FileSystem
Set LongPathsEnabled = 1 (REG_DWORD)
Let me know if that works (preferably without a reboot). If it works
without a reboot, we could make the change in the script to enable it and
then set it back when finished. I've actually run into issues with long
paths and driver downloads (can't remember the exact problem, think BITS
didn't like the length of the path or something). So it's possible this fix
could help a couple of things.

Reply to this email directly, view it on GitHub
https://github.com/rbalsleyMSFT/FFU/issues/80#issuecomment-2359018197,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABDS626V4S23SSGH6RC5XHDZXGYXDAVCNFSM6AAAAABONFH4DGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJZGAYTQMJZG4
.
You are receiving this because you authored the thread.Message ID:
<EMAIL_ADDRESS>

Reply to this email directly, view it on GitHubhttps://github.com/rbalsleyMSFT/FFU/issues/80#issuecomment-2359141481, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMYE2BGMCIWBHXPXGEDCHCDZXHAYTAVCNFSM6AAAAABONFH4DGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJZGE2DCNBYGE.
You are receiving this because you commented.Message ID<EMAIL_ADDRESS>

I already had to set this registry key a long time ago, so I didn't need to
set actually this time. Since I didn't have to change the value, I also
didn't reboot as there was nothing to "apply".
By the way, the New-ISOFile that I mentioned at the beginning is working as
far as I know as expected
On Wed, Sep 18, 2024, 8:34 PM rbalsleyMSFT @.***> wrote:

Did you try a reboot?
From: Laurent Louf @.>
Sent: Wednesday, September 18, 2024 11:27 AM
To: rbalsleyMSFT/FFU @.>
Cc: Richard Balsley @.>; Comment @.>
Subject: Re: [rbalsleyMSFT/FFU] oscdimg does not support long path (Issue
#80)
Seems like I forgot to mention that I did try that without success. The
register and the group policy (though they might be the same thing)
On Wed, Sep 18, 2024, 7:18 PM rbalsleyMSFT<EMAIL_ADDRESS>wrote:

Can you try setting LongPath support?
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\FileSystem
Set LongPathsEnabled = 1 (REG_DWORD)
Let me know if that works (preferably without a reboot). If it works
without a reboot, we could make the change in the script to enable it and
then set it back when finished. I've actually run into issues with long
paths and driver downloads (can't remember the exact problem, think BITS
didn't like the length of the path or something). So it's possible this
fix
could help a couple of things.

Reply to this email directly, view it on GitHub
https://github.com/rbalsleyMSFT/FFU/issues/80#issuecomment-2359018197,
or unsubscribe
<
https://github.com/notifications/unsubscribe-auth/ABDS626V4S23SSGH6RC5XHDZXGYXDAVCNFSM6AAAAABONFH4DGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJZGAYTQMJZG4
.
You are receiving this because you authored the thread.Message ID:
<EMAIL_ADDRESS>

Reply to this email directly, view it on GitHub<
https://github.com/rbalsleyMSFT/FFU/issues/80#issuecomment-2359141481>,
or unsubscribe<
https://github.com/notifications/unsubscribe-auth/AMYE2BGMCIWBHXPXGEDCHCDZXHAYTAVCNFSM6AAAAABONFH4DGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJZGE2DCNBYGE

.
You are receiving this because you commented.Message ID:
<EMAIL_ADDRESS>
—
Reply to this email directly, view it on GitHub
https://github.com/rbalsleyMSFT/FFU/issues/80#issuecomment-2359153976,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABDS622DX3B5OXZGFXXKFTLZXHBSRAVCNFSM6AAAAABONFH4DGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJZGE2TGOJXGY
.
You are receiving this because you authored the thread.Message ID:
@.***>

I think we need to do two things.

Enable Long Path support
Use \?. in front of the apps path

With just Long Path support enabled in the registry, I can get a script to create a path longer than 300 characters, but oscdimg fails to create the ISO. If I don't add the long path support, it doesn't create the complete directory path

If I add \?\ in front of the directory path to generate the ISO, it works.

Can you try modifying the New-AppsISO function by adding:
$appsPath = "\\?\" + $appsPath #Add the \\?\ prefix to the path to allow for long file names
before
Invoke-Process $OSCDIMG "-n -m -d $Appspath $AppsISO"
Seems to create the ISO for me without an issue. I don't have any apps though with the pathing issues you do, so it'd be good to know if this works for you. If it does, I'll need to add Long Path support plus this fix.

Thanks, glad to hear it's working.
Are you building on ARM64 devices?

No, just amd64

This has been fixed in 2412.1.


Petesong/sso token fix

Description
This adds RegistrationExpiresAt to the SSOToken that is returned when GetTokenAsync is called. This value is then used to generate a new SSO token if the registrationExpiresAt is within 5 minutes of expiration.
Motivation and Context
This change is for a better user experience within the VSToolkit for AWS
Testing
Local Testing. Dry run pending
Screenshots (if appropriate)
Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

[ ] My code follows the code style of this project
[ ] My change requires a change to the documentation
[ ] I have updated the documentation accordingly
[ ] I have read the README document
[x] I have added tests to cover my changes
[ ] All new and existing tests passed

License

[x] I confirm that this pull request can be released under the Apache 2 license

I cannot comment on the line directly since it's not part of the diff, but do we need to guard these locations against attempting to generate a new token with an expired registration as well?
https://github.com/aws/aws-sdk-net/blob/9be0493c0eafb30a7bec5f5ebc179a6eb12b2672/sdk/src/Core/Amazon.Runtime/Credentials/Internal/_bcl45%2Bnetstandard/SSOTokenManager.cs#L161
https://github.com/aws/aws-sdk-net/blob/9be0493c0eafb30a7bec5f5ebc179a6eb12b2672/sdk/src/Core/Amazon.Runtime/Credentials/Internal/_bcl45%2Bnetstandard/SSOTokenManager.cs#L353

I cannot comment on the line directly since it's not part of the diff, but do we need to guard these locations against attempting to generate a new token with an expired registration as well?
https://github.com/aws/aws-sdk-net/blob/9be0493c0eafb30a7bec5f5ebc179a6eb12b2672/sdk/src/Core/Amazon.Runtime/Credentials/Internal/_bcl45%2Bnetstandard/SSOTokenManager.cs#L161
https://github.com/aws/aws-sdk-net/blob/9be0493c0eafb30a7bec5f5ebc179a6eb12b2672/sdk/src/Core/Amazon.Runtime/Credentials/Internal/_bcl45%2Bnetstandard/SSOTokenManager.cs#L353

I don't believe so. In those cases, we're already going to request a new token, which performs a new client registration.

I cannot comment on the line directly since it's not part of the diff, but do we need to guard these locations against attempting to generate a new token with an expired registration as well?
https://github.com/aws/aws-sdk-net/blob/9be0493c0eafb30a7bec5f5ebc179a6eb12b2672/sdk/src/Core/Amazon.Runtime/Credentials/Internal/_bcl45%2Bnetstandard/SSOTokenManager.cs#L161
https://github.com/aws/aws-sdk-net/blob/9be0493c0eafb30a7bec5f5ebc179a6eb12b2672/sdk/src/Core/Amazon.Runtime/Credentials/Internal/_bcl45%2Bnetstandard/SSOTokenManager.cs#L353

I don't believe so. In those cases, we're already going to request a new token, which performs a new client registration.

yup in this case the new token would have a new client registration


DR: Functionality within data register to add new park/protected area

Description:
This ticket is to add functionality to the data register for program area to add new sub-areas themselves, without requiring a developer to manually update the register.

Will need to be split out into design/implementation
What other considerations might we want to include, given ongoing work with research into sub-areas (https://bcgov.sharepoint.com/:w:/r/teams/031072/Shared Documents/[Project] Parks Data Services/[Product] Parks Data Register/4. Research/Research findings/DR sub areas research findings.docx?d=waf8de1b3f20b4153872b97cec07668ff&csf=1&web=1&e=6ck6bx) and dates of operation research

Acceptance Criteria:
Development Checklist:

[ ] ...
[ ] ...
[ ] ...

Dependencies

Blocked by
Blocking

Relevant documentation as reference
Definition of Ready

[ ] Acceptance criteria are included
[ ] Wireframes are included (if applicable)
[ ] Design / Solution is accepted by Product Owner (if applicable)
[ ] Dependencies are identified (technical, business, regulatory/policy)
[ ] Story has been estimated (under 13 pts)

Definition of Done

In progress:

[ ] Acceptance criteria are tested (Functionality meets the acceptance criteria defined in the ticket)
[ ] UI meets accessibility requirements
[ ] Unit tests are written
[ ] Work is traceable in GitHub
[ ] PR linked to ticket number
[ ] If needed/required - Dev adds flag/label to highlight any migration steps necessary prior to PROD deployment

Code review:

[ ] Code is peer reviewed and has passed CI/CD tests

QA:

[ ] Acceptance criteria are tested (Functionality meets the acceptance criteria defined in the ticket)
[ ] Code is potentially shippable to the production environment
[ ] Functional features have been tested and passed by QA
[ ] UI components tested by designer
[ ] Code is deployed to PROD when moved to 'done' column (unless requested otherwise by PO)

PO Review:

[ ] Acceptance criteria are tested (Functionality meets the acceptance criteria defined in the ticket)
[ ] Reviewed and approved by Product Owner

Notes:

Refinement notes:

Glenna is the only one with permissions to add this stuff to the system
We would need to build a front end method for Glenna (or those with equal permissions) to add protected areas to the system.
Note: there's no api to add parks
each park and conservancy is defined by an orcs number which we don't have control over generating. challenging because this easily leads to human-error with manual-entry.

Curious on how many parks need to updated or added per year? What is the current cost of the team doing this vs building out a solution to allow others? Putting as low priority.


cli: Track terminal size to enable greater console history to be shown when possible

At the moment boardswarm-cli assumes the a default 80 column by 24 row terminal size. It is generally possible to resize a terminal window when running from a GUI and I tend to do this for at least the height, so that I can see a greater depth of the console history.
I note that this isn't always well supported, I tend to use picocom and launching cut down versions of vi in it can result in the terminal sizing being lost (until vi is closed and the terminal is resized, which seems to update whatever information in the TTY that vi changed), however this can be really really useful when trying to follow verbose processes.

Actually, width is as important to me. Long lines which wrap look horrible and are difficult to read.


Should the API talk more about locations (uri, position, ranges)

Testing #107585
The API is currently very generic except the navigation API. Since that already talks about uri, positions and ranges would it make sense to have an API that is tailored towards matches in files. That might make it a lot easier for implementors.

For example I have to provide a command right now to open an element in the tree.

That's how it initially started but the challenge is that this is locations (like reference search results) and symbols (like call hierarchy). They don't have much in common at the end of the day and that's why the API turned out being so generic


1 pixel gaps on no-gap settings

Steps to reproduce

gaps_in = 0
gaps_out = 0
border_size = 1
master layout
Expected outcome

no gaps between windows
Noted outcome

1px gap
it doesn't matter if i change window size

That 1 px is reserved for your border, to have absolutely no gap, set border_size = 0

It's not the border problem i guess. It really is noticeable with multiple windows

yes, each and every window is required to leave the required space for the border, this is not a bug, without it, the border of one app would simply overlap another application. Again, set your border_size to 0, to not allocate any space for possible borders.

@bbyeen does border_size=0 fix your issue?


[filter module] Fix bug

Quick fix for a bug in the B1 filtering module discovered by @MartinaBoshkovska.

Nice catch :) I've found the commit:
62b163410dd639c1ce1b6f48f07aa1e754c6721e

Coverage remained the same at 41.669% when pulling b2b0fc05e6ea06b344335f7dd7d2af43bc0b64f0 on b1_filter_bug into f8fe42eb5ca44ae925877feae5b4a88132976bed on master.


DUOS-716 Embed PowerBI dashboard in About page

Addresses
https://broadinstitute.atlassian.net/browse/DUOS-716
Screenshot:

Updated screenshot of full width + header:

Looks great!


Select button etc are enabled again after visibility of points layer is toggled

I am deactivating the select, add and remove button of the points layer, because they are not necessary or could lead to bugs in the case of add / remove.
However, after toggling the visibility of the points layer, these buttons are enabled again.
I tried to put in this check in on_layer_change, but it does not catch this, because it seems like toggling the visibility is not an event that triggers viewer.layers.events.changed.

Ah yeah, sorry about that. Setting the visible property also sets the editable property, which triggers the points layer controls to update the buttons.
We have this issue on the napari github about adding public methods to control which modes the user can access, so a fix is on the roadmap. I will link your issue there so we can be sure to cover it when we make the fix.
Thanks for the feedback!

Agreed, we should fix this, and thanks for making/ linking to that issue @kevinyamauchi!


nested count, aggregate _count query inefficiency

Bug description
When prisma builds count queries or aggregates for "_count" values why does it use IN to get values after joining when counting directly from the joined table would be much more efficient? Is there a certain reason to why it runs like this under the hood?
also I'm curious where the OFFSET came from. I'm guessing the offset value is 0 but I don't really understand why there is an OFFSET query here
you can see that using IN and OFFSET slows the query down a lot when you explain analyze the raw query

How to reproduce

Run count query with nested condition
Eg)

    await this.prisma.productMetrics.count({
      where: {
        product: {
          visible: true,
          approvedAt: { not: null },
        },
      },
    });

See the raw sql query that prisma runs
Eg)

SELECT count(*)
FROM (SELECT "public"."product_metrics"."product_id"
	  FROM "public"."product_metrics"
	  WHERE ("public"."product_metrics"."product_id") IN
	  	(SELECT "t0"."product_id"
		 FROM "public"."product_metrics" AS "t0"
                  INNER JOIN "public"."product" AS "j0"
                  ON ("j0"."id") = ("t0"."product_id")
		 WHERE ("j0"."visible" = 'true'
				AND "j0"."approved_at" IS NOT NULL
				AND "t0"."product_id" IS NOT NULL
                                 )
		)
	  OFFSET 0
	 )
	 AS "sub"

Expected behavior
I was expecting the code to count from the inner joined query straight up instead of using IN to match the product_ids again
SELECT count("t0"."product_id")
FROM "public"."product_metrics" AS "t0" INNER JOIN "public"."product" AS "j0" ON ("j0"."id") = ("t0"."product_id")
WHERE ("j0"."visible" = 'true'
AND "j0"."approved_at" IS NOT NULL
AND "t0"."product_id" IS NOT NULL)

Prisma information
model Product {
  id                          Int                 @id @default(autoincrement())
  name                    String
  visible                   Boolean             @default(true)
  approvedAt          DateTime?           @map("approved_at")
  ProductMetrics    ProductMetrics[]

  @@map("product")
}

model ProductMetrics {
  productId             Int     @id @map("product_id")
  product               Product @relation(fields: [productId], references: [id], onDelete: Cascade)
  dayReviewCount        Int     @default(0) @map("day_review_count")
  dayLikeCount          Int     @default(0) @map("day_like_count")
  viewCount             Int     @default(0) @map("view_count")

  @@map("product_metrics")
}

TS code
    await this.prisma.productMetrics.count({
      where: {
        product: {
          visible: true,
          approvedAt: { not: null },
        },
      },
    });

Environment & setup

OS: macOS
Database: PostgreSQL
Node.js version: v16.17.0

Prisma Version
4.12.0

why does it use IN to get values after joining when counting directly from the joined table would be much more efficient? Is there a certain reason to why it runs like this under the hood?

Probably just slipped through on our side when the feature was built - and because it returns the correct data, and no one has reported this as inefficient before, there was no need to revisit it.

also I'm curious where the OFFSET came from. I'm guessing the offset value is 0 but I don't really understand why there is an OFFSET query here

Probably similar reasons as above - using a OFFSET 0 when no offset is actually needed probably looked like a fair tradeoff to keep some code simple. Is the query faster without the OFFSET 0?

Thank you janpio for your detailed follow up with the bug!

Can you try the same query again with a newer version of Prisma, to make sure that this has not been fixed in the meantime?
Sure, I tried with 5.12.1 version and I found out that it has been fixed so that the query doesn't use the IN statement after joining(thx for the update! should have tried updating the version before reporting it mb) but I found out that the OFFSET statement was still there

SELECT COUNT(*)
FROM (SELECT "public"."product_metrics"."product_id"
	  FROM "public"."product_metrics"
	  LEFT JOIN "public"."product" AS "j1"
	  ON ("j1"."id") = ("public"."product_metrics"."product_id")
	  WHERE (
		  	 "j1"."visible" = 'true'
			 AND "j1"."approved_at" IS NOT NULL
			 AND ("j1"."id" IS NOT NULL)
			)
	  OFFSET 0
	 )
	 AS "sub"

Additionally, can you try with the relationJoins preview feature? That might also have impact on the generated SQL query and its performance.
I'm getting an error when I tried the relationJoins preview feature on the count query but it seems to be intended. I see that its omitted from the count query (If I'm using it wrong I would find it very kind of you if you could correct me thx!)

    await this.prisma.productMetrics.count({
      relationLoadStrategy: 'join',
      where: {
        product: {
          visible: true,
          approvedAt: { not: null },
        },
      },
    });

from index.d.ts
  type ProductMetricsCountArgs<ExtArgs extends $Extensions.InternalArgs = $Extensions.DefaultArgs> =
    Omit<ProductMetricsFindManyArgs, 'select' | 'include' | 'distinct' | 'relationLoadStrategy'> & {
      select?: ProductMetricsCountAggregateInputType | true
    }

Is the query faster without the OFFSET 0?
Comparing the two queries that I got from the updated version of prisma I did find out that including OFFSET did have an impact on the cost of the query.


ForAllInRange Fixes Yet Again

SUMMARY

Added individual container test cases for the 4 methods that are called by RoaringBitmap.forAllInRange
Fixed all the bugs I found with those.
Added more test cases for RoaringBitmap.forAllInRange and fixed all the bugs I found in those.

Automated Checks

[x] I have run ./gradlew test and made sure that my PR does not break any unit test.
[x] I have run ./gradlew checkstyleMain or the equivalent and corrected the formatting warnings reported.

Because this is a bug fix PR, and it adds new tests, and it is well prepared, I will merge and release a patch right away. This PR can't make things worse.


[NPU] two aclnn adaptation and two bf16 support and six tests.

aclnn: fill_   check_finite_and_unscale
bf16:   concat  elementwise

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.zhaojianqing-nj seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.You have signed the CLA already but the status is still pending? Let us recheck it.


Include iOS 16.7.6 - 16.7.8

The Developer Disk Image for iOS 16.4 should work fine for 16.7.6 to 16.7.8 as well.

Ah sorry bout that, didn’t notice 16 updated. Thanks for the pr.


Peek the connection to determine type of data on a CONNECT tunnel

Peek the connection to determine type of data on a CONNECT tunnel, rather than assuming based on the port of the connect request.
Added test to verify this is working with multiple requests on the same connection.

LGTM - @adamtanner want to take a look?

As a side note, this is the first step needed to make #31 work/not break.


Importing Material in non-root module causes build error

Bug, feature request, or proposal:
Bug
What is the expected behavior?
It is possible to import Material as a dependency of a module other than root.
What is the current behavior?
If Material is imported anywhere other than the application root, in an angular-cli project, the build fails with the following message (or similar depending on the used components).
can't resolve module ./autosize from<EMAIL_ADDRESS>(node:12240) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Cannot read property 'MdTextareaAutosize' of undefined, resolving symbol MdInputModule in<EMAIL_ADDRESS>resolving symbol MdInputModule in<EMAIL_ADDRESS>
Removing the import from the sub-module and attempting to only use an application-wide forRoot() import fixes the build error, but then results in runtime errors since the Material elements are no longer imported within the sub-module.
What are the steps to reproduce?
Use Angular-CLI to produce a project structure as seen here. (The problem is not reproducible via Plunkr itself.)
http://plnkr.co/edit/1TMZIIRhTRkaIMMVFMP2?p=preview
What is the use-case or motivation for changing an existing behavior?
Which versions of Angular, Material, OS, browsers are affected?
Angular 2.2.3, Material 2.0.0-alpha.11, angular-cli 1.0.0-beta.22, OS: Windows 7
Is there anything else we should know?
The problem did not happen under Material 2.0.0-alpha.10

Seems to be fixed using newest angular + material


Fix: Send payment receipt to client bugs

Notion Card
Notion Card
Description

Added default logo when no company logo was present. The default image I used is the same miru icon being used in the footer.
Show a currency symbol in front of the amount paid's amount.
Show the client name in the issued to column instead of a company name.

Preview
Before -

After -

@apoorv1316 I've made the requested changes. Please review.


HWKAPM-602 : Create hkTime filter for durations

Add an angular filter to convert from μs duration to human readable format.

@objectiser, thoughts ? Also, which other places should we apply it ?

@ammendonca Looks good - just need to change the column label to remove (ms) :)
Other places include:

Components view - the actual and elapsed columns
BTxn summary page - the 95% percentile value

Other related areas:

The min/max filter on the instance table - currently in milliseconds - so this be configurable, or fix on ms for now?
Duration scale on the charts (components and btxn details pages) - should they be sensitive to the max value?

@pavolloffay Thoughts?

In 95th percentile, the second unit is not displayed, as it breaks the UI with long values (eg: 123ms 456μs)

Looks good to me - @jpkrohling @pavolloffay comments?

How many chars are needed on the transactions part for the UI to break? If I have 10.000 transactions per hour (a very low number), will the UI break?

@jpkrohling it will take 10.000 transactions properly. even 100.000 and also 1.000.000 (which might need a small adjustment in terms of alignment, but won't break it). The thing is that the units add a lot more space and since they break to a new line (not breaking doesn't help much as it overlaps, see Booking Service) the whole card layout gets broken:

I think for these cards, just having a single unit may be ok. Also, might want to make the font size for the units smaller?

@objectiser, since the filter returns a whole string, making the units text smaller is not trivial and would likely involve mixing html code in the returned output, which I think is not very good practice. For that we'd likely want a directive instead.
I think for now it's ok with a single unit and same size, and in the future we can evaluate other options. If you agree, it's ready to be reviewed as it is.

@ammendonca Will start review, but looks like there is a lint error:
[ERROR] Error: Failed to lint: e2e/ts/e2ePlugin.ts[51, 1]: Exceeds maximum line length of 120, e2e/ts/e2ePlugin.ts[54, 1]: Exceeds maximum line length of 120, e2e/ts/e2ePlugin.ts[56, 1]: Exceeds maximum line length of 120, e2e/ts/e2ePlugin.ts[58, 1]: Exceeds maximum line length of 120.

@objectiser, oops, will fix.

@ammendonca Just tried running the vertx-opentracing example with ./order.sh joe laptop 1.
When I expanded the instance diagram, the table row at the top did not span the complete length of the diagram. Don't think this is related to this PR - possibly the properties one? So should I create a jira?
Also, one other place we could use the duration unit directive, on the distributed tracing and instance diagram nodes? Again, this could be a separate jira? or would you want to try it in this PR?

@ammendonca Have created https://issues.jboss.org/browse/HWKAPM-730 for the table width issue.
And https://issues.jboss.org/browse/HWKAPM-731 for discussing whether the same duration unit approach should be used on the diagram nodes.

@ammendonca Thanks!


Excel requires repair of rubyXL opened and saved xlsx

Opening this file and saving it in rubyXL produces errors in it that need to be repaired by Excel.
<?xml version="1.0" encoding="UTF-8" standalone="true"?>

-<recoveryLog xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">

<logFileName>error028200_01.xml</logFileName>

<summary>Errors were detected in file 'C:\Users\Kinesis\Downloads\Stockland_2013_GRESB_Report.xlsx'</summary>

-<removedRecords summary="Following is a list of removed records:">

<removedRecord>Removed Records: Named range from /xl/workbook.xml part (Workbook)</removedRecord>

</removedRecords>

</recoveryLog>

Any help would be greatly appreciated!

##Resolution:
I dug around in the XML and I have noticed this:

Some weirdness around the the definedNames:
[1]Lists!$B$1:$B$3

I removed this entry because it seems to be a referencing a file that doesn't exist

Issue may be related to referencing external files.
This issue can be closed.

It is a known issue. External links may go wrong because rubyXL does not track them properly yet. It requires a lot of work to fix, and it's in distant plans.

I ran into this issue and "fixed it" by:

opening the excel and allowing Excel to repair it
use the new repaired excel as the template for RubyXL to open

And the error just got fixed for new exports. I really don't know why but it worked.
Give it a try.
Duplicate of #230


raidemulator GetCombatants errors when seeking

Repro:

Import https://github.com/quisquous/cactbot/issues/3339#issuecomment-886120461
Open Trinity Avowed
(make a coffee)
Notice only Gleaming Arrow: expected 6, got 4 error in log (I'll look at this)
Click on 8 minutes in timeline
Hit play (?)
See a bunch of unexpected GetCombatants errors in the log

delubrum_reginae_savage.ts:2102 Avowed Avatar: expected at least 3 combatants got 0
delubrum_reginae_savage.ts:2358 Gleaming Arrow: expected 6, got 0
delubrum_reginae_savage.ts:2102 Avowed Avatar: expected at least 3 combatants got 0
delubrum_reginae_savage.ts:2358 Gleaming Arrow: expected 6, got 0
delubrum_reginae_savage.ts:2102 Avowed Avatar: expected at least 3 combatants got 0
delubrum_reginae_savage.ts:2358 Gleaming Arrow: expected 6, got 0

Actually, I suspect the expected 6, got 4 issue is also an emulator GetCombatants issue.  It's asking for these ids [1073760139,1073760140,1073760141,1073760142,1073760143,1073760144] but only getting 4 combatants back.

This if statement in the getCombatants override is failing the second condition for ID 0x4000478A:
        if (firstStateStamp > timestamp || lastStateStamp < timestamp)
          continue;

Looking into this further, it looks like because the state of the combatant is identical between casts of Gleaming Arrow (same position, same HP, etc), the new casts aren't being flagged as significant per this code:
https://github.com/quisquous/cactbot/blob/9495509340f72f75b500a8f87f8435c83cadc550/ui/raidboss/emulator/data/Combatant.ts#L96-L100
Should 20 lines force a significant state update, maybe?

Yeah, either force a significant state update or update the time of the previous one if it's the same?

We'd never want to update the previous timestamp, so it would be forcing a new state. Are there any other lines that should force a new significant state?

For context to help explain the logic here, "significant" states are combatant states where the combatant data is known to be completely accurate or states that no longer match the previous state. So if there are other lines where the combatant's state is guaranteed 100% accurate from the network log lines, it might make sense to have those force a significant state update as well.

Sorry, my mental model was that we "got next significant state" and so state A at time T and state A at time T=2 is equivalent to state A at time T=2 for "get next significant state".  Maybe I'm thinking about it wrong?  I think adding more significant states seems fine!


Add Migration field in network APIs

In 4.8, we want to add the Migration API in both network.config
and network.operator to better coordinate cluster network
migration process between MCO and CNO.

/cc @danwinship

/hold

/unhold

/cc @squeed

/approve

might want some updates to the docs but the API looks fine
/lgtm


linalg: copy subset of stablehlo-to-linalg passes from IREE

Jakub Kuderski did the wonderful work of migrating the MHLO-to-Linalg
conversion passes into IREE so that they translate from StableHLO to
Linalg, but these passes aren't available to non-IREE users of
StableHLO.  This patch copies Jakub's passes, with some small changes,
to enable a stablehlo-legalize-to-linalg pass in StableHLO.
This patch does not copy the IREE passes for lowering StableHLO's
control flow operations (while, if, and case) and the ones for
lowering shape computations, since I'm not sure whether lowering them
all in one pass (as opposed to separate passes) would impact IREE's
flow.  Whichever way we decide to add them, copying these remaining
translations should be trivial.

@burmako, I know you had started working on copying these passes, but since this is along a critical path for me, I went ahead and copied these passes from IREE.  Apologies for stepping on your toes!
I have left the copyright headers ("Copyright 2022 The IREE Authors") as they are, since it seems disingenuous to change them, given that Jakub did all of the work.  Let me know if I should add the standard OpenXLA copyright header ("Copyright 2022 OpenXLA Authors. All Rights Reserved.").
@kuhar, I noticed that the StableHLO control flow ops and shape computation ops are lowered in a separate pass.  Is that by design?  If I were to lower them all in one single pass, would that break IREE's transformations (or make them suboptimal)?

@kuhar, I noticed that the StableHLO control flow ops and shape computation ops are lowered in a separate pass. Is that by design? If I were to lower them all in one single pass, would that break IREE's transformations (or make them suboptimal)?

This was carried over from the MHLO-based code, I haven't put deep thought into this. Maybe @rsuderman or @jpienaar have some thought on this?

Thanks Eugene!  I'll need help fixing the Bazel build, since I know practically nothing about Bazel.

Rebased against the main branch to resolve the merge conflict.

Sorry, the Bazel error was probably from a bad merge.  Fixed it in the most recent revision.

@GleasonK whenever you get a chance, could you look at the changes please?  Thanks!

The code looks good. I needed to look into the licensing before approving. These files have a different license than the StableHLO repo, and I wanted to avoid a situation where all users of StableHLO transitively getting a new License dependency, or the need for an extern folder with a separate license to segment StableHLO license from this license.
That said, after some discussion - given that these files are all under the OpenXLA umbrella, this shouldn't be an issue to re-license. Given the CLA, a Googler must do this piece. I propose to accept these changes as-is, and I'll submit a PR with re-licensing prior to integrating these changes into openxla/xla. If that works for you then I'll merge these changes. Let me know if you have issues with that, otherwise will merge this EOD today and get started on the followup.

Thanks, that sounds good to me.  I have no issues with re-licensing.  I know it's a fair number of files that need updating, but hopefully something like a sed script will make that part trivial.

Can I make a request here (and I'm sorry if it adds burden, but I expect that your OSPO will approve it if asked): Can we retain the Apache-with-LLVM-Exception license for these as they originally had? Alternatively, I expect that if you ask, they will let you note in the license header that they are dual licensed under Apache2 and Apache2-with-LLVM-Exception.
The vast majority of code in this part of the MLIR ecosystem is Apache2-with-LLVM-Exception and it is a burden to not be able to share across the codebases.

Created https://github.com/openxla/stablehlo/issues/1848 so I don't forget. About to board a flight and don't want to forget about this comment - I'll take a look at this when I'm back in office. I don't think there should be any issues with your request, will raise with OSPO.


[Observer Decoder] Get rid of the required --gcs_bucket_name flag

for the elan_format_raw.py script. Instead, create a temporary bucket
on the fly if no bucket name is provided.
The documentation in the README.md file is updated accordingly.

Thanks for the review, @seanholmes77 !


inference code is use i-2K frames to predict i  but （-K to K） frames is used in paper，why difference？

OK，I misunderstanding the code，thanks for your work

@dlongry How to understand the way of using frames in code. I have the puzzle as you.
Can you tell us details?

You should see the "get_aggregation_symbol" function in the "\fgfa_rfcn\symbols\resnet_v1_101_flownet_rfcn.py": such as line 1057 ， use cur_data = mx.symbol.slice_axis(data_cache, axis=0, begin=cfg.TEST.KEY_FRAME_INTERVAL, end=cfg.TEST.KEY_FRAME_INTERVAL+1)  to get middle frame to detect。 @Feywell

@dlongry Thank you. So, it means it use frames from -k to k as paper.


Add rtg investigator

This tool won't work, I fear.

there are no dependencies declared, so nothing will be installed automatically
we have to figure out where to get the dependencies from, probably http://www.realtimegenomics.com/  but that's really messy
none of these tools have been touched since 2012.

Is there an alternative tool suite we can use? Can you share a bit about the use case for this?

I would like to compare VCFs and get precision and recall metrics. Previously I had done this using hap.py locally, however that one is not available in the toolshed. Its no problem I can also compare the VCFs locally.

hap.py is in bioconda, so we could wrap it up in a galaxy tool if it's helpful! If you have the command lines you want to run, we can start wrapping that and it'll be useful for everyone I reckon

Are you using hap.py with RTG? Or without, using xcmp?

This feature requires a version of rtg-tools which supports the GA4GH benchmarking intermediate file formats (see ga4gh/benchmarking-tools). Currently, such a version is available in this branch: realtimegenomics/rtg-tools@ga4gh-test

oh, probably not then

No I do not use it with RTG

Actually would you be up for learning how to wrap tools? It's honestly not too bad. You write a single XML file which describes the command line, and we can put it in Galaxy. (And then you aren't stuck waiting on me, to construct nice pipelines)

https://anaconda.org/bioconda/hap.py hap.py is in bioconda, so the really hard part is done
https://bio.tools/hap.py it has a bio.tools ID which is useful
All you'd have to do is try following this section of the tutorial to make the xml: https://training.galaxyproject.org/training-material/topics/dev/tutorials/tool-from-scratch/tutorial.html#galaxy-tool-wrappers
the full documentation is all here for hap.py https://github.com/Illumina/hap.py/blob/master/doc/happy.md#full-list-of-command-line-options

Or we can ask around, maybe Dennis or someone else has interest in learning about the wrapping process?

No I'm up for it, that only makes it easier for you and me and I might need to add more tools in the future. Thanks for the useful links Im gonna have a look at it!

Happy is now available in the toolshed