Service registry failures...

While reviewing a new PR for triggering discovery when config changes I ran into the following problem while testing.
When I build the latest version of device-rfid-llrp and run it against the latest/stable edgexfoundry snap, the device service is reporting that it cannot register to the registry because service information is not set.
Steps to reproduce:

Install edgexfoundry from latest/stable channel (i.e. Hanoi)

   $ sudo snap install edgexfoundry

Build this device service (note, it seems this service has already been updated to use go 1.16 even though it hasn't been fully updated to use V2 yet).

Run the service:

$ ./cmd/device-rfid-llrp-go --cp=consul.http://localhost:8500 --registry --confdir=./cmd/res
level=INFO ts=2021-06-15T17:37:57.568201468Z app=edgex-device-rfid-llrp source=config.go:193 msg="Loaded configuration from ./cmd/res/configuration.toml"
level=INFO ts=2021-06-15T17:37:57.569435976Z app=edgex-device-rfid-llrp source=config.go:168 msg="Using Configuration provider (consul) from: http://localhost:8500 with base path of edgex/devices/1.0/edgex-device-rfid-llrp"
level=INFO ts=2021-06-15T17:37:57.661931575Z app=edgex-device-rfid-llrp source=config.go:304 msg="Configuration has been pushed to into Configuration Provider (0 envVars overrides applied)"
level=INFO ts=2021-06-15T17:37:57.66202106Z app=edgex-device-rfid-llrp source=registry.go:80 msg="Using Registry (consul) from http://localhost:8500"
level=WARN ts=2021-06-15T17:37:57.662972521Z app=edgex-device-rfid-llrp source=registry.go:140 msg="could not register service with Registry: unable to register service with consul: Service information not set"
level=WARN ts=2021-06-15T17:37:58.665509302Z app=edgex-device-rfid-llrp source=registry.go:140 msg="could not register service with Registry: unable to register service with consul: Service information not set"
level=WARN ts=2021-06-15T17:37:59.667690141Z app=edgex-device-rfid-llrp source=registry.go:140 msg="could not register service with Registry: unable to register service with consul: Service information not set"
level=WARN ts=2021-06-15T17:38:00.669953378Z app=edgex-device-rfid-llrp source=registry.go:140 msg="could not register service with Registry: unable to register service with consul: Service information not set"
^Clevel=WARN ts=2021-06-15T17:38:01.672800492Z app=edgex-device-rfid-llrp source=registry.go:140 msg="could not register service with Registry: unable to register service with consul: Service information not set"
level=ERROR ts=2021-06-15T17:38:01.672854758Z app=edgex-device-rfid-llrp source=bootstrap.go:45 msg="aborted RegisterWithRegistry()"

According to @lenny-intel, this error comes from this code in go-mod-registry:
That error comes from here: https://github.com/edgexfoundry/go-mod-registry/blob/f0ab76939e05a341cddab2856861b590de3bce1f/internal/pkg/consul/client.go#L93-L95
One of there is not set:
client.serviceKey == "" || client.serviceAddress == "" || client.servicePort == 0 ||		client.healthCheckRoute == "" || client.healthCheckInterval == ""

@tonyespy I noticed that too. I think it is one of the healthcheck values. I looked around at various other EdgeX device services and had trouble finding where they are setting those values. @lenny-intel Do you have a good example working service I can model from for these values?

@ajcasagrande I'm looking at this now, and will let you know what I find out.

It looks like cmd/res/configuration.toml is missing Service.CheckInterval.

This is how we have it set in ASC:
https://github.com/edgexfoundry/app-service-configurable/blob/hanoi/res/rules-engine/configuration.toml#L20


fix: setting data root in CreateProposalBlock

Currently main fails when using celestia-app because the data root that is returned to it wasn't being correctly updated in the header.
This PR fixes this

Can a unit test verify this in celestia-core or is it only easily test-able from celestia-app?

I think we could have a unit test on this. At least to catch future regressions

does this break the PartSetHeader's hash?

I don't think so

perhaps a better question is simply, when are we planning on celestia-app using celestia-core main

Most likely for v3

TestByzantineConflictingProposalsWithPartition is failing in a way that I have no idea how it's connected to the changes

What seems to be happening is the following:
We have 4 validators, validator 0 is byzaninte, the remaining are divided into two partitions: Validator 1 in partition A and validator 2 and 3 in partition B.

Byzantine validator proposes two different proposals: Block A to partition A and Block B to partition B. It also sends prevotes to both.
Partition B reaches 2/3 and eventually produces a block
Network is healed (i.e. validator 1 can talk directly to validators 2 and 3)
Validator 1 receives 2/3 commits for B and tries to commit B. It sees that the proposal block parts it has (A) doesn't match what the 2/3 committed for (B) and panics

In the tests on main, it for some reason has nil for it's proposal block part so it prints something like:
commit is for a block we do not know about; set ProposalBlock=nil module=consensus validator=1 height=1 commit_round=0 proposal=343D64D40989F3C8A1CCB73D52AE0DAE8EEA1D51EB0443058DA8D9F84701D004 commit=C40D8D242C29FDB14E7B97A9D7C19751D980FF482AF13939BAC305906AF29553

It then sets it's block part header to B and eventually receives B and commits it.

does this break the PartSetHeader's hash?

I could be wrong, but it appears to since it removes the square size, which we're still including atm

does this break the PartSetHeader's hash?

I could be wrong, but it appears to since it removes the square size, which we're still including atm

I'm actually not sure that square size is populated in data (and in the block)

I'm actually not sure that square size is populated in data (and in the block)

unfortunately for us, we are on v0.34.x-celestia :sob: this was actually the reason why the compact blocks implementation didn't work in testground but did work in the comet e2e tests here
https://github.com/celestiaorg/celestia-core/blob/d3ff8e78962db594b3fea84de838bd2df2a2f7cd/types/block.go#L1097-L1122
we forgot to remove the square size, which makes upgrading to not using it more complicated than it needed to

Ah you're right. I was looking at main but I thought I was looking at v1.x. We can revert it and keep it around a bit longer.

do we need to update
https://github.com/celestiaorg/celestia-core/blob/d76e8443eebdda6179e95b301a4e6b4d9d39ff10/version/version.go#L20-L21
?

I don't think we have any block format changes since I reverted the protos to match v0.34.x-celestia


The script does not work correctly if channels count more than one

Hi.
How to embed (by channelid) two or more channels in one page? For example, this does not work correctly:

Because as a result, I see first channel - UCoiS7s7HrE7bHNuzERi-FCQ, but I do not see second channel - UCANLZYMidaCbLQFWXBC95Jg. How to fix that and make the script working for any number of videos?

Thanks,
you must use the class name to display multiple channels. 👍

Thank you for your quick reply to my request! It works 👍
But I have another question: how to embed YouTube channels without Increasing page size, as described (for video) in this article: http://www.labnol.org/internet/light-youtube-embeds/27941/ ?

Thanks a lot @apachx , a very good idea. now I have to fix it.
I have changed the code css and javascript. This will work if autoplay = false.
Best regards, Ican.

It works perfectly now. Thank you! That's really cool.


fix:Avoid accessing static methods of a class through its object refe…

Avoid accessing static methods of a class through its object reference.

@wankai123 I think this is caused by last PromQL support?


[docs site] small typo in docs site for Map.has

(about https://www.totaltypescript.com/ts-reset)
small fix! the [2, "waqas"] member in these 2 code snippets should be ["waqas", 2], re: the supposed error message on the key mismatch:
// Argument of type '"bryan"' is not assignable to
// parameter of type '"matt" | "sofia" | "waqas"'.

Fixed, thanks!


smooth_ui_library null safety

@monsieurtanuki, thanks for your review, the problems should be fixed now.

Will it be compatible with smooth_app

I don't see any reason why it shouldn't be compatible. All packages in smooth_app now support null safety. (Not just in theory, all the packages on the null safety version.)
Actually I had planned to migrate smooth_app and smooth_ui_library together, but smooth_app is much bigger and I haven't looked at a large parts of the models and database files.

@monsieurtanuki, interesting the android studio analyzer didn't marked it before a restart

@monsieurtanuki, interesting the android studio analyzer didn't marked it before a restart

I think that's the limit of the migration in auto-pilot mode. Of course most of the time the automatic upgrade assumptions will be decent, but especially for public methods there's the added value of the developer - or of the code-reviewer ;)
Combined with the added value of null-safety.

@M123-dev That's what I meant with compatibility worries between smooth_app and null-safe smooth_ui_library:

@M123-dev My bad. I've forgotten to pub get smooth_ui_library too.
Don't see the point of two separate subprojects and pubspec.yamls though...

@monsieurtanuki I understand your concerns, it complicates the development process a bit. But I don't think it's worth restructuring the whole project for this.


Section styles are not uniform

E.g. \section font sizes are bigger than the header

No longer applicable/has been addressed


Encrypted Client Hello indicator

This has been a great indicator to keep informed which protocol is in use for a particular site. Can a new indicator be incorporated to indicate if Encrypted Client Hello is in use? The lightning colors could stay the same but the icon could be updated to add a secure symbol in the same color if ECH is in use. Cloudflare just enabled ECH for many sites as per its blog post last week so ECH will start to be used more and more. Since it is privacy related it is very helpful for the user to know if it is in use when visiting a website.
See the ECH section in the Cloudflare blog on Sep 27 2024 - https://blog.cloudflare.com/new-standards/

Sounds good, is there any browser API we can read this property?
For reference, we use nextHopProtocol to get the current HTTP protocol.


Use chained list datastructure in hash join

Is your feature request related to a problem or challenge?
No response
Describe the solution you'd like
No response
Describe alternatives you've considered
No response
Additional context
No response

Closed by #6679


[CELEBORN-470][FLINK] support stopTrackingAndReleasePartitions

What changes were proposed in this pull request?
1、support stopTrackingAndReleasePartitions when celeborn workers were not available.
Why are the changes needed?
When encounter celeborn worker shutdown, data would be lost, Flink tasks will be retry but still try to read the lost data, but the job would not success. So we need to let the Flink framework known which data of the partitions can't be reading anymore, then Flink will schedule these partitions to regenerate
Does this PR introduce any user-facing change?
No
How was this patch tested?
TPCDS with random worker kill

@waitinfuture I merge ShuffleWorkerStatusListener with ShuffleResourceTracker, and change untrack partitions by shuffle to  job for we can notify all untracking partitions to flink framework at one time.


ERROR: some URL forbidden for chaturbate.com

i have problem with chaturbate.com now. this is log (for any model):
youtube-dl --verbose https://chaturbate.com/modelname/
[debug] System config: []
[debug] User config: []
[debug] Custom config: []
[debug] Command-line args: ['--verbose', 'https://chaturbate.com/modelname/']
[debug] Encodings: locale cp1251, fs mbcs, out cp866, pref cp1251
[debug] youtube-dl version 2018.02.26
[debug] Python version 3.4.4 (CPython) - Windows-10-10.0.15063
[debug] exe versions: ffmpeg N-89794-gc51301db14, ffprobe N-89794-gc51301db14, rtmpdump 2.4
[debug] Proxy map: {}
[Chaturbate] modelname: Downloading webpage
ERROR: Unable to download webpage: HTTP Error 403: Forbidden (caused by HTTPError()); please report this issue on https://yt-dl.org/bug . Make sure you are using the latest version; type  youtube-dl -U  to update. Be sure to call youtube-dl with the --verbose flag and include its complete output.
  File "C:\Users\dst\AppData\Roaming\Build archive\youtube-dl\rg3\tmpfqykkd7x\build\youtube_dl\extractor\common.py", line 519, in _request_webpage
  File "C:\Users\dst\AppData\Roaming\Build archive\youtube-dl\rg3\tmpfqykkd7x\build\youtube_dl\YoutubeDL.py", line 2199, in urlopen
  File "C:\Python\Python34\lib\urllib\request.py", line 470, in open
  File "C:\Python\Python34\lib\urllib\request.py", line 580, in http_response
  File "C:\Python\Python34\lib\urllib\request.py", line 508, in error
  File "C:\Python\Python34\lib\urllib\request.py", line 442, in _call_chain
  File "C:\Python\Python34\lib\urllib\request.py", line 588, in http_error_default

Export cookies with solved captcha from browser and provide to youtube-dl.


correction in dartboard plot generation

only a visual aspect after all the other changes. Results are not affected.


Can ListItem use TouchableOpacity instead of TouchableHighlight?

I looked through the documentation and cheatsheets.
If I want ListItems to have that TouchableOpacity effect instead of TouchableHighlight, how can that be customized?
That seems it would have to switch what the ListItem is wrapped with.
Is this just something we can't customize?

I think you can't customize, you can only update the ListItem component to use TouchableOpacity instead of TouchableHighlight

Hi @Aryk, yes, you cannot customize. For Android, it is TouchableNativeFeedback and for iOS & web, it is TouchableHighlight by default.
But you can make changes manually in node_nodules/native-base/src/basic/ListItem.js from TouchableHighlight to TouchableOpacity.

@hanykumar You can but how would I make those changes carry over into production? Or when I re-install node modules?
Really... there should be an ability to pass a way to overwrite the component with your own TouchableHighlife / TouchableNativeFeedback.
For example...if I want it to use TouchableHighlight from react-native-gesture-handler library...how can I do that without something super hacky?

@Aryk There isn't any way to do this in NativeBase. But to achieve this, you can just wrap a ListItem of NativeBase with TouchableOpacity, that should do the trick.
Closing this for now, let us know if you need further clarification/help on this.


Update-DbaInstance Failed to get reboot status Patching SQL 2016

Hi Team,
I am getting this error, while doing patching test to the remote server.
PS C:\Windows\system32> $ServerList = 'SQL_servername'
Get-DbaBuildReference   -SqlInstance $ServerList |ConvertTo-Html |out-file "D:\Source\SQL_Patch\2016\SP2\CU12\CU_info.htm"
#create a credential to pass in to the Update-DbaInstance command; this will prompt for your password
#$cred = Get-Credential DOMAIN\your.account
PS C:\Windows\system32> D:\Source\SQL_Patch\Scripts\SQL2016_patch_test.ps1
VERBOSE: [12:44:10][Resolve-DbaNetworkName] Resolving SQL_servername using .NET.Dns GetHostEntry
VERBOSE: [12:44:10][Resolve-DbaNetworkName] Resolving <IP_ADDRESS> using .NET.Dns GetHostByAddress
VERBOSE: [12:44:10][Get-DbaCmObject] Configuration loaded | Cache disabled: False
VERBOSE: [12:44:10][Get-DbaCmObject] [SQL_servername] Retrieving Management Information
VERBOSE: [12:44:10][Resolve-DbaNetworkName] Unable to get domain name from SQL_servername
VERBOSE: [12:44:10][Resolve-DbaNetworkName] Resolving SQL_servername using .NET.Dns GetHostEntry
WARNING: [12:44:12][Update-DbaInstance] Error while looking for SQL Server installations on SQL_servername | Connecting to remote server SQL_servername failed with the following error message : WinRM cannot process the request. The following error with errorcode 0x80090311 occurred while using Kerberos authentication:
There are currently no logon servers available to service the logon request.
Possible causes are:
-The user name or password specified are invalid.
-Kerberos is used when no authentication method and no user name are specified.
-Kerberos accepts domain user names, but not local user names.
-The Service Principal Name (SPN) for the remote computer name and port does not exist.
-The client and remote computers are in different domains and there is no trust between the two domains.
After checking for the above issues, try the following:
-Check the Event Viewer for events related to authentication.
-Change the authentication method; add the destination computer to the WinRM TrustedHosts configuration setting or use HTTPS transport.
Note that computers in the TrustedHosts list might not be authenticated.
-For more information about WinRM configuration, run the following command: winrm help config. For more information, see the about_Remote_Troubleshooting Help topic.
PS C:\Windows\system32>

For testing, i just created a local login with admin access, it been working. Issue with domain account, could be a Kerberos.

Got it, it is working<EMAIL_ADDRESS>


Archive binary journal

We need to create the persistent journal directory first, so as one of the last steps when building your AMI do mkdir -p /var/log/journal. I didn't see where you were building the AMI.
Then, to read the contents of this archive extract it and journalctl -D .

I'm not sure how portable this format is, but I'd assume a fedora journalctl can read a rhel7 journal.


Spring Boot 3.2.x / Spring Cloud 2023.0.0 - bitbucket issues with Spring Cloud Kubernetes during maven test runs

Under e.g. Spring Boot 2.x / Spring Cloud Hoxton.SR10 I found that running maven tests as part of a BitBucket pipeline ran into issues where Spring Cloud automatically detected the presence of Kubernetes in the BB environment and tried to e.g. gather kubernetes ConfigMaps as property sources, leading to unexpected NamespaceResolutionFailedException when running tests.
In these circumstances this could be fixed by passing spring.cloud.kubernetes.enabled=false as a system property when invoking maven (e.g. via a -D switch to the maven command line, or within the systemPropertyVariables configuration of the maven-surefire-plugin).
After uprevving to Spring Boot 3.2.1 / Spring Cloud 2023.0.0, I found that this mechanism no longer worked and I was getting NamespaceResolutionFailedException again on BB maven test runs.
I manged to fix the problem by adding a src/test/resouces/bootstrap-kubernetes.yml file in which Spring Cloud kubernetes was set disabled.
Is it expected that the previous mechanism no longer works, or is it more likely that I have misconfigured something?

Is this really a duplicate? That linked bug seems to be mostly focused on earlier versions of SB/SC.
This is about SB 3.2.x / SC 2300.0.0.
And I may be missing something, but its not at all clear to me that the resolution (which seems to be "closed due to lack of feedback") to that issue sheds any light on this one.

Is this really a duplicate? That linked bug seems to be mostly focused on earlier versions of SB/SC.

I assume you would get the same error using SB 3.1.x and SC 2022.0.x
Are your tests expected to pass even when not running on a K8S cluster?

Yes that's right - the tests expect to pass with no dependency on k8s being present, and this is how they operated in SB 2.x. To this end, spring cloud kubernetes was disabled via a system property.
Under SB 3.2 it appears that this property no longer prevents spring cloud from attempting to locate e.g. ConfigMap property sources if it detects the presence of k8s (as is the case when running unit tests in BitBucket).
I have found an alternative approach as mentioned in the issue text above; whether this is a "bug" is moot - I'm more concerned that I understand whether this apparent change in behaviour is expected, and whether there is a better way of achieving what I need.

Yes this was a breaking change in 2022.0.x
https://docs.spring.io/spring-cloud-kubernetes/docs/3.0.4/reference/html/#breaking-changes-in-3-0-x
Are you loading ConfigMap using Bootstrap or using spring.config.import?

Ah thanks!
I'm using Bootstrap (setting spring.cloud.kubernetes.config.sources in src/main/resources/bootstrap.yml).
Previously I was disabling Spring Cloud kubernetes for unit tests using this configuration of the maven surefire plugin:
<systemPropertyVariables>
  <spring.cloud.kubernetes.enabled>false</spring.cloud.kubernetes.enabled>
</systemPropertyVariables>

That breaking change explains why this no longer works, so I guess I need to replace it with:
<systemPropertyVariables>
  <spring.main.cloud-platform>NONE</spring.main.cloud-platform>
</systemPropertyVariables>

I've looked again in the linked issue, but it's not leaping out at me :-)
But in any case I'm happy that I now understand what changed and how to adjust to that change - thank you both for your help.

Glad it’s clear now!


feat: New areas added

PR Details
Areas added based on the new GitHub labels and some emojis changed.

Added: area-Core, area-Math, area-Serialization
Emojis changed: area-Graphics, area-UI, area-Rendering

Related Issue
N/a
Types of changes

[x] Docs change / refactoring / dependency upgrade
[ ] 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

[x] My change requires a change to the documentation.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.
[ ] I have built and run the editor to try this change out.

Thanks a bunch for looking into this, forgot to mention that rendering has been removed since it overlaps with graphics ...

You are welcome. No problem, let me update it :)

Done. You can merge it whenever 🥳.

Thanks !


Pathfinding is bugged when getting a new path while the entity is already following a path. This is because since paths aren't instantly solved, the entity will actually walk backwards to get to the start of the next path, not considering that it has moved since it requested the path.

The solution is to iterate through all the new nodes and only start at the one the entity is closest to. This should prevent most instances of walking back to the start of the new path.

Fixed


[stlab]Upgrade to 1.5.1

Related: #8792.
Note: this port does not contain any features.

/azp run

/azp run


Simplify Showtime Component, Accept Single Child

Overview
Previously the Showtime component took a function as a child, which took a ref and attached it to the wrapped component. That function is now moved to be inside the library, simplifying the client API.
The way the ref is attached to the child element is by cloning the element and attaching the ref to the clone. This may have some performance penalty, depending on the complexity of the component. This may also have other consequences, say if the client code is tracking that component otherwise (e.g. by using a ref of their own).
Demo
Previously:
<Showtime show={show}>
  {(ref) =>
Oh hi
}
</Showtime>

Now:
<Showtime show={show}>

Oh hi

</Showtime>

Notes
There is also a lint commit which was an artifact of my editor removing trailing spaces. If they are meaningful spaces, I can drop that commit to keep them in.
Testing Instructions
I don't have a great sense of the edge cases and performance implications described above. Generally I prefer simplicity to performance, and think this will help ease adoption of the project. In my limited testing it seemed to work fine, but would appreciate those more familiar with this project run it through the ringer.
Not sure how best to test this. Just ensure that it still works correctly with the new API, I guess.

Another impact that cloneElement may have is if using a really complex component, say like a Map, React may not be able to treat the diffs uniformly, causing the entire component to re-render. I haven't verified if that is the case, but if so, then we should abandon this PR.

Might it be possible we could have the best of both worlds by accepting either a render prop function or jsx. The component would inspect the sole child. If it's a function, we assume it's a render prop and call it, passing in the ref parameter. If it's an object, then we assume it's jsx and clone it, with the ref as a new prop.
This way developers can pass in jsx, but if in any given instance this leads to a performance problem, they can pass in a render prop instead and hopefully 🤞 that fixes it.

We should probably accommodate inbound refs, so they also get attached instead of overridden. Here's how to do it.
Related: Readme instructions to developers for attaching their own refs along with the Showtime ref.

Thanks for the tip, I'll take a look later today.
Regarding the best of both worlds, I sometimes feel that it's better to have one way of doing things in the library than multiple, makes it easier to understand as a user. I'll keep it in mind.

better to have one way of doing things in the library than multiple

Generally agree, and it may be that's the best answer in this case. But a couple of points in favor of the dual solution:
Easy defaults: Passing JSX should "just work" most of the time. Hopefully you can mostly forget about the function version, and just rustle it up when necessary.
Precedence: React and other libraries often overload their APIs with different parameter types for special cases. A nearby example is useState's lazy initialization, where instead of an initial value, you can pass a function that gets evaluated only on the first render.

Getting back to this now

A nearby example is useState's lazy initialization, where instead of an initial value, you can pass a function that gets evaluated only on the first render.

That's a neat trick!!
Just pushed up a change that has the overloaded API. Also adjusted the docs to reflect that. Please take a look and recommend any needed changes. Thanks!

Thanks for all the suggestions and testing! I squashed the fixups, will merge once green.


Fixes formatting of calicoctl config kdd table

Description

Fixes the formatting of the KDD calicoctl config table, making it consistent with the etcd table

Todos

[ ] Tests
[ ] Documentation
[ ] Release note

Release Note

None required

also check reference/felix/configuration.md for similar changes.

🌵


[BUG] Optimizely track Event returns error TypeError: undefined is not an object (evaluating 'this._client')

Is there an existing issue for this?

[X] I have searched the existing issues

SDK Version
2.9.2
Current Behavior
I have defined an sdk Instance in my react app like below
`
const optimizelyInstance = createInstance({
        sdkKey,
        datafileOptions: dataFileDefaultValues,
        eventBatchSize: 100,
        eventFlushInterval: 3000,
      });
      setLogLevel(config.logLevel || LogLevel.ERROR);

// Set the Optimizely client in the state
optimizelyClientRef.current = optimizelyInstance;`

this is then exported as a hook called useOptimizelyContext like so
export const useOptimizelySDKContext = () => {
  const { optimizelyClient, getOptimizelyUser, user, setUser } = useSafeContext(
    OptimizelySDKContext
  );

  if (!user) {
    throw new Error("User is not set");
  } else {
    return {
      getVariation: optimizelyClient?.getVariation,
      activate: optimizelyClient?.activate,
      setUserAttributes: setUser,
      isFeatureEnabled: optimizelyClient?.isFeatureEnabled,
      isReady: optimizelyClient?.isReady,
      getOptimizelyUser,
      user,
      optimizelyTrackEvent: optimizelyClient?.track,
      optimizelyClient,
    };
  }
};

However the track Function keeps returning undefined, I tried accessing optimizelyClient.track and it returns undefined.
so if I do
const { optimizelyClient, optimizelyTrackEvent } =
useOptimizelySDKContext();
const onClickAction = () => {
optimizelyTrackEvent("event_name") // fails since optimizelyTrackEvent is undefined
}
Expected Behavior
I should be able to use the track function to send events from optimizely for my ab tests.
Steps To Reproduce
I have defined an sdk Instance in my react app like below
`
const optimizelyInstance = createInstance({
        sdkKey,
        datafileOptions: dataFileDefaultValues,
        eventBatchSize: 100,
        eventFlushInterval: 3000,
      });
      setLogLevel(config.logLevel || LogLevel.ERROR);

// Set the Optimizely client in the state
optimizelyClientRef.current = optimizelyInstance;`

this is then exported as a hook called useOptimizelyContext like so
export const useOptimizelySDKContext = () => {
  const { optimizelyClient, getOptimizelyUser, user, setUser } = useSafeContext(
    OptimizelySDKContext
  );

  if (!user) {
    throw new Error("User is not set");
  } else {
    return {
      getVariation: optimizelyClient?.getVariation,
      activate: optimizelyClient?.activate,
      setUserAttributes: setUser,
      isFeatureEnabled: optimizelyClient?.isFeatureEnabled,
      isReady: optimizelyClient?.isReady,
      getOptimizelyUser,
      user,
      optimizelyTrackEvent: optimizelyClient?.track,
      optimizelyClient,
    };
  }
};

However the track Function keeps returning undefined, I tried accessing optimizelyClient.track and it returns undefined.
so if I do
const { optimizelyClient, optimizelyTrackEvent } =
useOptimizelySDKContext();
const onClickAction = () => {
optimizelyTrackEvent("event_name") // fails since optimizelyTrackEvent is undefined
}
React Framework
v18
Browsers impacted
No response
Link
No response
Logs
No response
Severity
Blocking development
Workaround/Solution
No response
Recent Change
No response
Conflicts
No response

Thanks for the report @leojacob7.
I'll do an initial review and try to set up a scenario based off your example.
Real quick: Is optimizelyClient defined and has the ready promise resolved by the time your custom hook is consumed?

I think I went deep down and figured out that the optimizelyClient infact exists, so I am doing a const optimizelyInstance = createInstance({sdkKey}) and if I try to access optimizelyInstance.track("eventName") it seems to give me access to track and I do get a warning saying user context not set which i acceptable, however if I wrap this up inside the optimizelyProvider and try to use it inside the component wrapped inside I get an error saying TypeError: undefined is not an object (evaluating 'this._client')

Sorry about the delay. I'm trying to get v3 out 😁
You're correct. Using the JavaScript SDK directly should give you all the access you need to track().
Let me spin up a test project and see if I can reproduce your result using the OptimizelyProvider

Hiya @leojacob7 I tried setting up a React app to mimic your scenario based on the info from your above submission.  Can you take a look and correct what I'm missing?
https://github.com/mikechu-optimizely/react-sdk-implementation/tree/react-sdk-issue-211
PR me. 😁

Hey @leojacob7 Any thoughts on my previous 👆


282 discover all nested objects

Q
A

Is bugfix?
✔

New feature?
❌

Breaks BC?
❌

#282
#286

Add tests for nested structures.
Update buildObjectsCache() to discover all nested objects only if required.
4.1 Update dumpNestedInternal() to stop traverse deeper if object is not found in the cache. This prevents possible errors in the future.
4.2 Fix dumpNestedInternal() to not stop dumping objects on the level 2*$depth.

Yet another test:

Both objects must be present in the expected result.

Both objects must be present in the expected result.

No. Depth is 1, so second object don't must be in result.

Both objects must be present in the expected result.

No. Depth is 1, so second object don't must be in result.

It's a trap.
https://github.com/yiisoft/yii-debug/issues/282#issuecomment-2454144914

Explanaition of buildObjectsCache()
In this PR, the buildObjectsCache() creates different $this->objects caches for asJsonObjectsMap() and asJson() methods.
For the asJsonObjectsMap() method with @vjik's improvements, it caches all possible objects from the $variable.
So this condition in the dumpNestedInternal() will never fire and can be removed indeed.
https://github.com/olegbaturin/yii3-debug/blob/7ca18453c342f027fa0c4bd7ab55c42b873ce3f0/src/Dumper.php#L174
Additional calls of the buildObjectsCache() from the dumpNestedInternal() also have no sense.

Proposal for Callables dumping
Callables are dumped as strings and not as objects.
https://github.com/olegbaturin/yii3-debug/blob/7ca18453c342f027fa0c4bd7ab55c42b873ce3f0/src/Dumper.php#L159
But in the $this->objects cache Callable objects are still presented, and they are dumped without links to them from the parent properties.
"Yiisoft\\Definitions\\CallableDefinition#149": {
	"private $callable": "static fn (\\Yiisoft\\Aliases\\Aliases $aliases) => new \\Yiisoft\\Mailer\\MessageBodyTemplate(\n    $aliases->get($params['yiisoft\/mailer']['messageBodyTemplate']['viewPath']),\n)"
},
"Closure#147": "static fn (\\Yiisoft\\Aliases\\Aliases $aliases) => new \\Yiisoft\\Mailer\\MessageBodyTemplate(\n    $aliases->get($params['yiisoft\/mailer']['messageBodyTemplate']['viewPath']),\n)",

The proposal is to skip Callable objects in the buildObjectsCache() method.
"Yiisoft\\Definitions\\CallableDefinition#149": {
	"private $callable": "static fn (\\Yiisoft\\Aliases\\Aliases $aliases) => new \\Yiisoft\\Mailer\\MessageBodyTemplate(\n    $aliases->get($params['yiisoft\/mailer']['messageBodyTemplate']['viewPath']),\n)"
},

Proposal for Callables dumping

Can we make it in separate PR?

Is it good to go?

After discussion with @olegbaturin, we have suggestion.

Rename $depth parameter in asJsonObjectsMap() method to $arrayDepth.
$arrayDepth will be mean output depth for array object properties only.
And therefore, pass $depth+2 instead of $depth+1 in asJsonInternal().

@xepozz WDYT?

Proposal for Callables dumping
Callables are dumped as strings and not as objects. https://github.com/olegbaturin/yii3-debug/blob/7ca18453c342f027fa0c4bd7ab55c42b873ce3f0/src/Dumper.php#L159
But in the $this->objects cache Callable objects are still presented, and they are dumped without links to them from the parent properties.
"Yiisoft\\Definitions\\CallableDefinition#149": {
	"private $callable": "static fn (\\Yiisoft\\Aliases\\Aliases $aliases) => new \\Yiisoft\\Mailer\\MessageBodyTemplate(\n    $aliases->get($params['yiisoft\/mailer']['messageBodyTemplate']['viewPath']),\n)"
},
"Closure#147": "static fn (\\Yiisoft\\Aliases\\Aliases $aliases) => new \\Yiisoft\\Mailer\\MessageBodyTemplate(\n    $aliases->get($params['yiisoft\/mailer']['messageBodyTemplate']['viewPath']),\n)",

The proposal is to skip Callable objects in the buildObjectsCache() method.
"Yiisoft\\Definitions\\CallableDefinition#149": {
	"private $callable": "static fn (\\Yiisoft\\Aliases\\Aliases $aliases) => new \\Yiisoft\\Mailer\\MessageBodyTemplate(\n    $aliases->get($params['yiisoft\/mailer']['messageBodyTemplate']['viewPath']),\n)"
},

Why?

After discussion with @olegbaturin, we have suggestion.

Rename $depth parameter in asJsonObjectsMap() method to $arrayDepth.
$arrayDepth will be mean output depth for array object properties only.
And therefore, pass $depth+2 instead of $depth+1 in asJsonInternal().

@xepozz WDYT?

I don't mind if the name changed
Why $depth+2 instead of $depth+1?

Why?

It's for your evaluation. If orphan lines for closures "Closure#147": "..." are required, just decline this proposal.

Variant 1. Consistent to to asJson() method
Level 0:
"array (1 item) [...]"

Level 1:
{
    "stdClass#2512": "stdClass#2512 (...)"
}

Level 2:
{
    "stdClass#2512": {
        "public $name": "nested1",
		"public $var": "array (1 item) [...]"
    }
}

Variant 2. Based on object level.
Level 0:
{
    "stdClass#2512": "stdClass#2512 (...)"
}

Level 1:
{
    "stdClass#2512": {
        "public $name": "nested1",
		"public $var": "array (2 items) [...]"
    }
}

Level 2:
{
    "stdClass#2512": {
        "public $name": "nested1",
		"public $var": [
			"test",
			"array (2 items) [...]"
		]
    }
}

Variant 3. Based on arrays.
Level 0:
{
    "stdClass#2512": {
        "public $name": "nested1",
		"public $var": "array (1 item) [...]"
    }
}

Level 1:
{
    "stdClass#2512": {
        "public $name": "nested1",
		"public $var": [
			"test",
			"array (2 items) [...]"
		]
    }
}

Level 2:
{
    "stdClass#2512": {
        "public $name": "nested1",
		"public $var": [
			"test",
			[
				"hello",
				"world"
			]
		]
    }
}

Does a result of asJsonObjectsMap() with collapsed objects make sense:
{
    "stdClass#2512": "stdClass#2512 (...)"
}

?
If no, we can use "Variant 3. Based on arrays." or "Variant 1. Consistent to to asJson() method", but limit minimum value of $depth to two.


Simple styling with tailwind

I know this is probably a big ask here, but it would be great for this package to be infinitely customizable an work with more modern frameworks like tailwind.
As of now its really hard to update any of the styles

Hello davidnoah
To make the package infinitely customizable and compatible with modern frameworks like Tailwind CSS, you could consider the following approach :
Move Styles to Configuration or Theme Files for exemple :
const defaultStyles = {
button: 'bg-blue-500 text-white px-4 py-2 rounded',
card: 'shadow-md border rounded-lg p-4',
};
export const getStyles = (customStyles) => ({
...defaultStyles,
...customStyles,
});
than  Integrate Tailwind Compatibility
Instead of hardcoding styles, use utility classes from Tailwind CSS.
Provide a way for users to pass their own Tailwind classes.
Example:
const MyComponent = ({ customClasses = '' }) => {
return (
<div className={p-4 bg-gray-100 ${customClasses}}>
{/* Component Content */}

);
};
3. Expose a Configuration API
Add an API that lets developers inject their own theme or Tailwind classes.
You can allow them to pass a configuration object at initialization.
for example:
import { configure } from 'your-package';
configure({
theme: {
button: 'bg-green-500 hover:bg-green-600 text-white font-bold',
},
});
4. Provide a Default Tailwind Theme
Offer a pre-built Tailwind configuration as the default theme.
This makes it easier for users already using Tailwind.
Example:
const tailwindTheme = {
button: 'bg-blue-500 hover:bg-blue-600 text-white font-medium',
input: 'border rounded px-3 py-2 focus:ring focus:ring-blue-300',
};
Good Luck


Investigate only bundling required database connectors

Would probably involve modifying prisma-engines

Is there a workaround for this right now?
Takes ages to download from within docker

@John0x Not currently, I have to modify prisma-engines to do so and just don't have time to do it right now. You could try utilising docker layer caching and building primsa-client-rust separately to your other dependencies with cargo build -p prisma-client-rust.

Gonna put this on hold again, I just don't have time atm

I just opened PR prisma/prisma-engines#3157 about separating the sql and monogdb features. I will be interested if they merge it because the Prisma Engines already had a convention to feature gate sql vs monogodb but it just wasn't done properly everywhere.

This now works on the feature-gate-dbs branch. Will be doing testing to see if it's suitable for release.

72dbb636473d3b93ede4fc99361b4cb8533dd72b


The box and wire model with ports and components (WIP)

This is my initial sketch of how the model could look like after ui model split.
Posted as PR for feedback, still lacks most methods and comments.

This looks very reasonable :+1: I can easily imagine building a graph of these, and I think it neatly represents everything we've got at the moment and everything we're planning to add.
I'm not yet sure where type checking (or even turning stuff into expression trees) would fit in, if at all, but that's mostly blocking on me grokking the connection-based type checking.

What should be modifiable and not are open questions, and it not clear in the current ui code either.
And what methods are exactly needed depends on results expression graph stuff and other refactorings.
A big question for me is whether the model needs to/should refer back in some way to UI things.

Yep. The introduction of these classes would be a good moment to settle who is allowed to change what and when.
I'm afraid that either model code will have to reference UI elements or vice versa, even if you'd ideally have neither.

Commits in modelsplit can also be found in modelsplit-wn.


how config or set the charset

[root@appserver ~]# df -h
ÎÄ¼þÏµÍ³              ÈÝÁ¿  ÒÑÓÃ  ¿ÉÓÃ ÒÑÓÃ%% ¹ÒÔØµã
/dev/mapper/VolGroup-lv_root
507G  153G  328G  32% /
tmpfs                  16G  1.2M   16G   1% /dev/shm
/dev/xvda1            485M   52M  408M  12% /boot
<IP_ADDRESS>:/MyWork
985G  223G  712G  24% /MyWork
the “¹ÒÔØµã” cannot display，is china word

Well, there's you're problem, the code isn't supported in Chinese word.

I accept pull requests, the code is there, the modules I'm using are published (it's probably either needing to pass an environment variable via ssh2. or a setting for xterm. I don't have experience working with other character sets so I'm not really the right one for that job.


Training loss curve on V2

You updated to V2 version but it only support 1 batch size. How much computation cost you do when compared to V1?
I also want to see the loss curve so since I started to training it with only 1 A100 and loss for AE task is about ~2.xx

In the pretrain, using the Wikipedia dataset and using the learning rate of 1e-4 can help jump out of the local optimal solution, and the loss can be reduced to 0.xx

@Zyvpeng thanks for the response. I see in the paper you mention that loss is under 0.1
So 0.xx here should be 0.0x?
I have made progress in training it but it looks like only about 0.4x

@toilaluan When I train ICAE with lm_ratio=0, the loss can reach under 0.1. However, when I set  lm_ration=0.4, I face the same problem as you, so what's your lm_ratio? By the way, did the author mention anything about lm_ratio? Thank you!


Error with exporting large data image

So fig.write_image works just fine on doing distplot on a dataframe of a few 100k rows. However, when I pass the 1 million rows threshold it outputs the "plotly cannot communicate with orca sometimes" error.
Is there any workaround for this?
Thanks in advance!
-Roham

have u encountered this error?
"Error: The orca command-line utility is required for this functionality." I dont know what else I can do ! thanks

Hi, I have the same issue. Here is the error message when I try to export a large plot:
For some reason plotly.py was unable to communicate with the
local orca server process, even though the server process seems to be running.

Please review the process and connection information below:

orca status
-----------
    state: running
    executable: /Users/guillaumerozier/opt/anaconda3/bin/orca
    version: 1.3.0
    port: 50438
    pid: 90835
    command: ['/Users/guillaumerozier/opt/anaconda3/bin/orca', 'serve', '-p', '50438', '--plotly', '/Users/guillaumerozier/opt/anaconda3/lib/python3.7/site-packages/plotly/package_data/plotly.min.js', '--graph-only', '--mathjax', 'https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.5/MathJax.js']

Any updates? I have the same issue. Doing choropleth maps with pff.create_choropleth(), scope is USA counties, I make a map each day for about 110 days. A large list of binning_endpoints seems to trigger the crash, but it depends on the data - in some cases it's fine with thousands of endpoints, in other cases 200 crash it.
Dataframe has one column for each county, one row for each day.
import plotly.figure_factory as pff
import os

def make_images(arglist):
    df, endpts, map, type, scale = arglist
    datelist = df.index.tolist()
    for ddate in datelist:
        dlist = ddate.split('/')
        datenow = '20' + dlist[2] + dlist[0].rjust(2, '0') + dlist[1].rjust(2, '0')
        datenowslash = '20' + dlist[2] + '/' + dlist[0].rjust(2, '0') + '/' + dlist[1].rjust(2, '0')

        fig = pff.create_choropleth(
            fips=df.columns.tolist(),
            values=df.loc[ddate],
            scope=[map],
            county_outline={'color': 'rgb(0,0,0)', 'width': 0.5},
            binning_endpoints=endpts,
        )

        fig.update_layout(
            autosize=False,
            width=2000,
            height=1066,
            showlegend=False,
            title={
              'text': '' + map.upper() + ' BIG MAP
' + type + '
' + scale + '
' + datenowslash + '',
              'y': 0.2,
              'x': 0.1,
              'xanchor': 'left',
              'yanchor': 'middle'},
        )

        imgpath = os.path.join(type.replace(' ', '_'), 'frame-' + datenow + '.png')

        fig.write_image(imgpath, scale=1.0)

I have several workers with that function, one for each CPU. After dozens of images, they start to crash.
---------------------------------------------------------------------------
RemoteTraceback                           Traceback (most recent call last)
RemoteTraceback:
"""
Traceback (most recent call last):
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/plotly/io/_orca.py", line 1562, in to_image
    figure=fig_dict, format=format, scale=scale, width=width, height=height
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/retrying.py", line 49, in wrapped_f
    return Retrying(*dargs, **dkw).call(f, *args, **kw)
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/retrying.py", line 212, in call
    raise attempt.get()
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/retrying.py", line 247, in get
    six.reraise(self.value[0], self.value[1], self.value[2])
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/six.py", line 703, in reraise
    raise value
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/retrying.py", line 200, in call
    attempt = Attempt(fn(*args, **kwargs), attempt_number, False)
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/plotly/io/_orca.py", line 1477, in request_image_with_retrying
    raise OSError("522: client socket timeout")
OSError: 522: client socket timeout

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/florin/opt/anaconda3/lib/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/Users/florin/opt/anaconda3/lib/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/Users/florin/notebook/covid19us/workers.py", line 35, in make_images
    fig.write_image(imgpath, scale=1.0)
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/plotly/basedatatypes.py", line 2824, in write_image
    return pio.write_image(self, *args, **kwargs)
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/plotly/io/_orca.py", line 1770, in write_image
    fig, format=format, scale=scale, width=width, height=height, validate=validate
  File "/Users/florin/opt/anaconda3/lib/python3.7/site-packages/plotly/io/_orca.py", line 1594, in to_image
    info=status_str
ValueError:
For some reason plotly.py was unable to communicate with the
local orca server process, even though the server process seems to be running.

Please review the process and connection information below:

orca status
-----------
    state: running
    executable: /Users/florin/opt/anaconda3/bin/orca
    version: 1.3.1
    port: 50094
    pid: 1574
    command: ['/Users/florin/opt/anaconda3/bin/orca', 'serve', '-p', '50094', '--plotly', '/Users/florin/opt/anaconda3/lib/python3.7/site-packages/plotly/package_data/plotly.min.js', '--graph-only', '--mathjax', 'https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.5/MathJax.js']

"""

The above exception was the direct cause of the following exception:

ValueError                                Traceback (most recent call last)
<ipython-input-12-475aa2260917> in <module>
     15     # call workers
     16     work_out = p.map(make_images, usabsarglist)
---> 17     work_out = p.map(make_images, usrelarglist)
     18     p.close()

~/opt/anaconda3/lib/python3.7/multiprocessing/pool.py in map(self, func, iterable, chunksize)
    266         in a list that is returned.
    267         '''
--> 268         return self._map_async(func, iterable, mapstar, chunksize).get()
    269
    270     def starmap(self, func, iterable, chunksize=None):

~/opt/anaconda3/lib/python3.7/multiprocessing/pool.py in get(self, timeout)
    655             return self._value
    656         else:
--> 657             raise self._value
    658
    659     def _set(self, i, obj):

ValueError:
For some reason plotly.py was unable to communicate with the
local orca server process, even though the server process seems to be running.

Please review the process and connection information below:

orca status
-----------
    state: running
    executable: /Users/florin/opt/anaconda3/bin/orca
    version: 1.3.1
    port: 50094
    pid: 1574
    command: ['/Users/florin/opt/anaconda3/bin/orca', 'serve', '-p', '50094', '--plotly', '/Users/florin/opt/anaconda3/lib/python3.7/site-packages/plotly/package_data/plotly.min.js', '--graph-only', '--mathjax', 'https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.5/MathJax.js']

It's failing because of a timeout stop_max_delay=60000. Not sure why that's there.
In case it's helpful, this workaround does the trick for me.
# Disable the orca response timeout.
import plotly.io._orca
import retrying
unwrapped = plotly.io._orca.request_image_with_retrying.__wrapped__
wrapped = retrying.retry(wait_random_min=1000)(unwrapped)
plotly.io._orca.request_image_with_retrying = wrapped

I had the same issue on MacOS and it was caused because the electron app (that launches on executing fig.write_image) was interrupted. Restarting my IDE and rerunning the script solved the issue.


How were the normalized scores aggregated?

Thank you for releasing the code! This is a very interesting piece of work. Congrats on the NeurIPS acceptance! 🎉
As per my understanding, you're aggregating normalized scores to report the final scaled score. It looks like you're using the arithmetic mean to aggregate the normalized scores. Please correct me if I am wrong.
Using the arithmetic mean may not be the best way of summarizing a normalized metric. This may lead to misleading conclusions. A better way to aggregate normalized scores is using the geometric mean. Please check this paper out for details:
Fleming, Philip J., and John J. Wallace. "How not to lie with statistics: the correct way to summarize benchmark results." Communications of the ACM 29.3 (1986): 218-221.
Based on the numbers in https://github.com/ngruver/llmtime/blob/main/precomputed_outputs/deterministic_csvs/monash.csv, here are the plots that I get using the arithmetic and geometric mean.

Thanks for the note Abdul!
The reported values are an arithmetic mean and you're correct that this is probably suboptimal. Genuine apologies for the error on my part.
I am planning to update the arxiv with extended experiments from our NeurIPS camera-ready and I'll include this correction as well.
Please let me know if you have any other comments.
Nate


Failed to uninstall plugin using lower case.

To Reproduce
Steps to reproduce the behavior:

Uninstall plugin using lower case, like uninstall applicationlogs
It returns Uninstall successful, but actually the plugin still exsits.

Screenshots

Source Code
https://github.com/neo-project/neo-node/blob/master/neo-cli/CLI/MainService.Plugins.cs#L217
Here delete plugin folder with Plugins/applicationlogs, but it should be Plugins/ApplicationLogs.
Platform:

OS: [ubuntu 18.04]

I think I have fixed this in mode pr

I think I have fixed this in mode pr

No, you haven't :disappointed_relieved:


Treemacs colorful icon theme

Given it's a little heavy change, I decided it was better to assign a reviewer. This change adds a doom-themes-treemacs-color-icons flag that when true, displays a more colorful Treemacs theme than the original, minimal atom inspired implementation by Henrik.

Looks good, but you've introduced tabs indentation where previously it's all spaces. Please untabify!

Sorry, didn't noticed that. Done

Thanks, looks good to me!  This just needs someone to install it and double-check that it's working.  I haven't managed to do that tonight, will do it tomorrow if it's still pending.

@flatwhatson don't worry! Thanks for reviewing it!

Testing it out, I'm not sure if the usability don't is quite right.  I expected the following to work:

set doom-themes-treemacs-color-icons to t
run M-x load-theme and load/reload a doom theme
see pretty new icons in treemacs

Instead, I need to do the following:

set doom-themes-treemacs-color-icons to t
run M-x treemacs-load-theme and select a different theme
run M-x treemacs-load-theme and select the doom theme
see pretty new icons in treemacs

@hlissner should we force a treemacs theme reload when loading a doom theme?

@flatwhatson the thing is that the the treemacs theme is loaded independently from the themes (because it's optional), so there's no way that the treemacs theme is loaded automatically when loading a doom theme

Looking at this again, I think the problem is that the whole treemacs-create-theme call needs to be re-evaluated after toggling doom-themes-treemacs-color-icons.  This isn't easily possible at runtime, it just runs once when this module is loaded.
The obvious solution that comes to mind is to go back to the idea of having two completely separate treemacs themes, with the doom-themes-treemacs-color-icons variable simply deciding which of those is loaded by default.  Then users can still switch at runtime using treemacs-load-theme.

Let's make Henrik decide! I still think this is the best option given that I don't think people will switch treemacs themes in runtime, but let's see what he thinks

OK, though I'm not sure if we really need Henrik here.  If you split into two themes, I'll merge it.
A simple use-case for changing theme at runtime is: I want to compare the two before making a final decision.  Currently that requires editing config, saving, restarting emacs... much less convenient than the couple of calls to treemacs-load-theme it should be.
While the current version is more compact, it's not worth sacrificing usability IMO.

I implemented my own colorful themes based on `all-the-icons- already.
Refer to https://github.com/seagle0128/.emacs.d/blob/master/lisp/init-ui.el#L161. The extensions are not hard-coded.

@seagle0128 That's very nice and compact! The thing with our implementation is to have granularity in order to have as much control as possible with the icons given that there are certain modes or file types (i.e. Verilog) that have icon but the icon isn't associated with the file type or the mode in all the icons

@flatwhatson sorry I forgot to answer! I think I'll wait for Henrik since we have opposite opinions so I think he can decide what's best!

@ema2159 IMO, it's better leveraging all-the-icons-icon-alist and all-the-icons-mode-alist, but it needs making big changes for some macros and functions of treemacs. @hlissner doesn't want to change them for now.
Of course the current changes also can make it and I glad to integrate to Centaur.

Any update for this PR?

I'm waiting for @hlissner, he was going to be a little busy this week

@seagle0128 how do you use all this: https://github.com/seagle0128/.emacs.d/blob/7d14f4c7336f4901f066a5ab484372098c9b8447/lisp/init-ui.el#L139 without getting a Keyword argument ... not one of (:icon-directory :config :extends) error?

@ema2159 I didn't use treemacs-create-icon for buffer icons, since the macro treemacs-create-icon doesn't support this case. Refer to
https://github.com/Alexander-Miller/treemacs/issues/504#event-2520987027. So I leverage the hash table inside of treemacs-create-icon to implement it.

@seagle0128, so you modified the Treemacs macro in order to work that way? Such a shame, we were going to use your implementation and just add the corner cases after because it was so much cleaner!!!

@ema2159 Not exactly. I didn't modified the macro directly, while extracted and leveraged the inside codes to make it. You can compare my implementation and treemacs-create-icon macro.

I was missing a (progn jaja, it works smoothly now

Perfect!

@ema2159 I've done some small cleanup and revised how the treemacs theme is configured.
Tested that the theme looks goood, and can be switched at runtime with treemacs-load-theme.
I think this is ready to go, so if you're happy with my changes please go ahead and squash it!

@flatwhatson great! I'll do it today!

@seagle0128 there ya go, now we have a cool colorful Treemacs theme. Thanks for your code snippet, it made the implementation smoother an cleaner

@ema2159 Welcome! It looks great, and I've already updated my configurations.


Docs and tests: Struct deprecation for Any and hosts deprecation for load_assignment

Description: update docs and tests for Struct -> Any and hosts -> load_assignment. This should complete the docs, examples, sandboxes and testing configs. TODO is to update the code tests.
Risk Level: Low, documentation and tests
Testing: bazel test //test/...
Docs Changes: XDS protocol, Zookeeper filter & flow control
Release Notes: None required
Related: #6346

/cc @dio @htuch I didn't update https://github.com/envoyproxy/envoy/blob/master/test/config/integration/server.json which is a config that is very out of date. I couldn't find a way to trigger this in tests and for example could delete sections of the JSON to cause invalid JSON and all tests would pass. The references for server.json are https://github.com/envoyproxy/envoy/search?q="server.json"&unscoped_q="server.json". If this config is note being used at all can we just remove it? (converting server.yaml was a huge undertaking)

/retest

@moderation if you find a config that is not used and not tested, definitely delete it. Can you confirm that all configs that you have been fixing are covered by some test? I just want to make sure that we don't have a test bug somewhere that is causing a config to not get tested.

/wait
Not stale, just haven't had a chance to work on this this week.

Still not stale.


Roll Flutter Engine from 11502404a52a to aeff9b174c84 (2 revisions)

https://github.com/flutter/engine/compare/11502404a52a...aeff9b174c84
2024-05-13<EMAIL_ADDRESS>Migrate FlutterDartProject and FlutterPluginAppLifeCycleDelegate to ARC (flutter/engine#52719)
2024-05-13<EMAIL_ADDRESS>Roll Skia from dde1f5dba183 to 75b3286ecaac (1 revision) (flutter/engine#52763)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC rmistry<EMAIL_ADDRESS>on the revert to ensure that a human
is aware of the problem.
To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Closing to trigger a new roll past a framework revert.

Commit queue failed; closing this roll.


Build of new SettingsSchemaKey API fails

https://travis-ci.org/sdroege/gstreamer-rs/jobs/309075030#L1030-L1041
error[E0425]: cannot find function `g_settings_schema_key_ref` in module `ffi`

  --> /home/travis/.cargo/git/checkouts/gio-a4b825b310a073b9/c704556/src/auto/settings_schema_key.rs:17:27

   |

17 |         ref => |ptr| ffi::g_settings_schema_key_ref(ptr),

   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^ did you mean `g_settings_schema_ref`?

error[E0425]: cannot find function `g_settings_schema_key_unref` in module `ffi`

  --> /home/travis/.cargo/git/checkouts/gio-a4b825b310a073b9/c704556/src/auto/settings_schema_key.rs:18:29

   |

18 |         unref => |ptr| ffi::g_settings_schema_key_unref(ptr),

   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ did you mean `g_settings_schema_unref`?

cc @EPashkin

You use feature v2_40 ?

--all-features actually (to make sure none of my features are conflicting with each other and they all always compile). It happens when using gio with features = ["v2_38"] or earlier. v2_40 works fine as that will pull in the same feature from gio-sys.
The problem here is that the whole type is v2_40, but only its impl part is hidden behind that. The glib_wrapper! macro is not.

You're never supposed to run --all-features. Maybe we should add a check to see if more than one version feature is enabled at once to make the compilation fail (we now have a compilation_fail!` macro so wy not using it?).

Why make something fail that generally works unless you break things?
Also again, this breaks also if you just enable a pre-2.40 feature on gio. This does not need --all-features to fail.

@sdroege Gio.SettingsSchemaKey don't have version in Gio-2.0.gir file,
so gir try detect it automatically by function, and some of function (g_settings_schema_key_get_description) have "2.34".
Can you try set version in config and check failure again.
Fix can be included in regen with https://github.com/gtk-rs/gir/pull/498

You mean
[[object]]
name = "Gio.SettingsSchemaKey"
status = "generate"
version = "2.40"

That has no effect

Strange.
It don't change generated code?
cargo build --all-features on fixed gio works fine for me.
cargo build --features="v2_34" too.

How does "fixed gio" look like? And yes, inside gio I also can't get it to fail for whatever reason. But create an empty new project with this in the Cargo.toml and it will fail: gio = { git = "https://github.com/gtk-rs/gio", features = ["v2_38"] }

Key changes is
diff --git a/src/auto/mod.rs b/src/auto/mod.rs
index 8f4df03..fdf691c 100644
--- a/src/auto/mod.rs
+++ b/src/auto/mod.rs
@@ -1,4 +1,4 @@
-// This file was generated by gir (d50d839) from gir-files (469db10)
+// This file was generated by gir (83cd11e+) from gir-files (469db10)
 // DO NOT EDIT

 mod action;
@@ -101,9 +101,9 @@ pub use self::resource::Resource;
 mod settings_schema;
 pub use self::settings_schema::SettingsSchema;

-#[cfg(any(feature = "v2_34", feature = "dox"))]
+#[cfg(any(feature = "v2_40", feature = "dox"))]
 mod settings_schema_key;
-#[cfg(any(feature = "v2_34", feature = "dox"))]
+#[cfg(any(feature = "v2_40", feature = "dox"))]
 pub use self::settings_schema_key::SettingsSchemaKey;

Empty project with "v2_38" also builds fine for me.

Yes that change would work

Strange that you don't get it generated by https://github.com/gtk-rs/gio/issues/67#issuecomment-348545925

Well, let's merge this then and make my travis green again? :)

This? I don't see any PR for gio.

I thought you submitted one :)

No, I still waiting your PR's with https://github.com/gtk-rs/gir/pull/498.
so I though that you can add this change to gio's.

Not really important to do regens for that one, and as I wrote above the change in the .toml file does not work for me while it does for you. I must be adding something wrong to the .toml file.

IMHO any update that change many files in many crates better applied fast to simplify check future updates.
https://github.com/gtk-rs/gio/issues/67#issuecomment-348545925 is look right change for me,
may be you forgot to remove "Gio.SettingsSchemaKey" from generate array and gir just don't do anything.


state vars / units of info do not have consistent 'class' fields

https://github.com/PathwayCommons/sbgnml-to-cytoscape/blob/master/src/sbgnmlConverter.js#L161
For some reason, they have a  'clazz' field instead of 'class'.

                "statesandinfos": [
                    {
                        "id": "13a",
                        "clazz": "state variable",
                        "state": {
                            "value": "P"
                        },
                        "bbox": {
                            "x": -1354.6949631131793,
                            "y":<PHONE_NUMBER>4565056,
                            "w": "25.0",
                            "h": "22.0"
                        }
                    },
                    {
                        "id": "13b",
                        "clazz": "unit of information",
                        "label": {
                            "text": "mt:prot"
                        },
                        "bbox": {
                            "x": -891.9979688146107,
                            "y": 1108.3777362198236,
                            "w": "53.0",
                            "h": "18.0"
                        }
                    }
                ],

Does the label subobject need to exist?  Can it just have label: 'string'?


fix build failed in the pytorch branch

@coronaPolvo is a catalina user. He pushed some .DS_store files that caused build failed into the branch.

already down

#83 fixed this


Clone golang.org/x/build
Settings:

"gopls": {
        "ui.documentation.linksInHover": "gopls",
}

Open maintner/github.go, and hover over *github.Label in line 418.
Click "github.Label in gopls doc viewer" link.

What did you see happen?
"package not found" on browser
What did you expect to see?
Package doc
Editor and settings
"gopls": {
        "ui.documentation.linksInHover": "gopls",
}

VS Code
Logs
No response

Related Issues

x/tools/gopls: package (still) not found when "linksInHover": "gopls" #68223
x/tools/gopls: negated build flag causes error #59346
x/tools/gopls: add a setting not to download modules #39264 (closed)
x/tools/gopls: unable to find package #38271 (closed)
x/tools/gopls: not working #37119 (closed)
x/tools/gopls: “no metadata for <package>” in workspaces setup #56470 (closed)
x/tools/gopls: broken hovering in the builtin file #68026 (closed)
x/tools/gopls: undeclared name when accessing symbols from other files #35337 (closed)
x/tools/gopls: references doesn't work properly. #60676 (closed)
x/tools/gopls: improve error message when using wrong go command with Go source distribution #35520

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

It's possible that #68223 can be the same issue.
The issue is present in gopls v0.16 too.

Thanks, I can reproduce the problem. The URL is lacking the final /github segment of the package path. If you instead use Source Action > Browse Documentation, it computes the correct URL. Will investigate.

Ah, the logic in golang.formatLink is to blame:
	if pkgURL != nil { // LinksInHover == "gopls"
		path, _, _ := strings.Cut(h.LinkPath, "@") // remove optional module version suffix
		url = pkgURL(PackagePath(path), h.LinkAnchor)

LinkPath is<EMAIL_ADDRESS>and the Cut yields github.com/google/go-github/v48.

Seems like an easy fix, thanks Alan for investigating. I'll put this in the v0.17.0 milestone, but we can of course fix it later.


feat(stepper): Make CdkStepper generic to support extended CdkStep

Feature Description
Currently, when I want to use CdkStepper with a custom class for a step (extension of a class CdkStep, e.g. MyStep extends CdkStep), I am not able to as CdkStepper is not generic and is fixed to CdkStep class for a step. I would like to make CdkStepper generic, so I can use CdkStepper<MyStep>.
Use Case
Let's say I have a classOnboardingStepComponent extends CdkStep which has the property specialLabel. I want to use the CdkStepper with this specialized step component.
Let me know if this sounds reasonable, if so, I can work on this.

We also had faced same issue, we worked around by defining new getter like
class BespokeStepper extends CdkStepper{
    get selectedStep() {
        return this.selected as MyStep;
    }
}

And using that in template


Generic dataclass detection fails for unions

Hi Brent,
I have a very low-level bug to flag for you -- when saving/loading nested dataclasses to yaml (using extras.to_yaml(), extras.from_yaml(), if a dataclass has a Union of two custom types, they don't get detected as custom types for the yaml.Loader to construct.
I wrote a MWE to replicate the issue:
import dataclasses
import dcargs

from typing import Union

@dataclasses.dataclass
class TypeA:
    data: int

@dataclasses.dataclass
class TypeB:
    data: int

@dataclasses.dataclass
class Wrapper:
    subclass: Union[TypeA, TypeB] = TypeA(1)

if __name__ == "__main__":
    wrapper1 = Wrapper() # Create Wrapper object.
    wrapper2 = dcargs.extras.from_yaml(Wrapper, dcargs.extras.to_yaml(wrapper1)) # Errors, no constructor for TypeA

No worries if this is too low-level to deal with right now -- I think we can work around it by just pickling the configs, but wanted to flag something is going awry in the custom type detection.

Wow thanks so much for the lightning-quick patch :)

oops, seems it was too fast and the CI is broken :)
looking into it...

Actually, everything seems to be in order; just had a typo in a test + an outdated version of typing_extensions locally.
Appreciate the issue report + reproduction instructions! As an FYI I am thinking of deprecating the API though, I can think of a couple other issues and the benefit over just using PyYAML seems fairly incremental. Will file a separate issue.

Thanks, appreciate the heads-up! I wanted to file an issue in case the Union typing issue went further than just the YAML utils. Before your patch, we were just pickling the dataclass objects with no problem, we might just stick with that.

Sounds good!
If you haven't considered it, I'd recommend also looking into serializing with PyYAML + the standard yaml.load(your_yaml, Loader=yaml.Loader) and yaml.dump(your_dataclass_instance) helpers. It should be basically as flexible as pickle, bigger file sizes but still human-readable + editable.


Make aggregations easier

Maybe Repo.aggregate(query, column)?

We should also consider aggregate_by that does a group_by + aggregate.

Repo.aggregate(query, :count) #=> PK
Repo.aggregate(query, :sum, :salary)

Repo.aggregate_by(query, [:age], :count, :id)
#=> %{27 => 3, 28 => 6, ...}

In master!


Same-origin-policy // CORS issue with Soundcloud Resolve API? (FF)

I'm hitting Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://api.soundcloud.com/resolve?url=http://soundcloud.com/giraffage&format=json&consumer_key=xxx. (Reason: CORS header 'Access-Control-Allow-Origin' missing). on a live site as well as localhost in Firefox 38. Any idea what could be wrong? Hitting a brick wall here, your demo page seems to work fine.

I did notice that a new version of Chrome was causing trouble, showing CORS errors when we were trying to set up the visualizer. I took out the visualizer from the player and I haven't seen the issue since. Are you using a self-hosted version of the player?


Azure Web PubSub tile missing from hub page

The Azure Marketing page at https://azure.microsoft.com/services/#integration lists Web PubSub in the Integration category. The service isn't listed at all on the Azure docs hub page's Integration section. These two pages should be in sync.

@scottaddie, Thanks for reporting this. The Azure Web PubSub service should be listed under "Web" -https://azure.microsoft.com/services/#web. It looks like it's listed on both places ( Integration + Web). So the entry on Azure doc hub page  needs to be updated - @yjin81, requesting you take a look into this.

Hey,
I am new to this area and need your help about a question.
We submitted the request to put the AWPS on Web when onboarding the ACOM. Later, when onboarding the marketplace, we found that this is separate from ACOM, and we could onboard on different places at the same time to help us attach with the customer. Therefore, we submitted to add AWPS to both Web and Integration in Marketplace. At that time, my understanding is that doc should keep align with ACOM.
Would you please let me know whether I should make these 3 places (ACOM, doc and marketplace) the same or could be different?
Thanks
Yan

#reassign:grminch

This is fixed. PubSub is listed under Web and Integration.
#please-close


Config sigs for standard detector and standard flyer should use the same naming, same type

Currently in StandardDetectors we call them config_sigs or _config_sigs, and internally they are in a list. In the StandardFlyer, they are configuration_signals and are a tuple. We should settle on one or the other for both things.
In addition, the type hint for the flyer wants a sequence of SignalR, while for the standard detector a sequence of AsyncReadable.
Also, as mentioned in #563 , for StandardDetectors the signals that we typically want in config_sigs are instantiated with the detector, meaning they need to be set/updated after the device is initalized. Currently, you can do this with det._config_sigs =  or something similar, but we should probably find a way to do this without needing to access the private attribute.

Do we have a usecase for the ones in StandardFlyer yet? Otherwise I'm tempted to delete them until we have a usecase for them

I have not used them personally. I suppose all devices should be able to set their config sigs independently, so these are not really needed?


Argument meaning.

hello! what's the meaning of "<your_wandb_run_name>"

alpaca_farm connects to wandb by default for tracking training curves. If you don't care about this you can put whatever you want there.

okok,thank you!


Move BuildTreeObjectFactory to private package

Fixes https://github.com/gradle/gradle-private/issues/4262
Before merging the PR, comments starting with

❌ ❓must be fixed
🤔 💅 should be fixed
💭 may be fixed
🎉 celebrate happy things

@bot-gradle test this

I've triggered the following builds for you. Click here to see all build failures.

Pull Request Feedback (Trigger) (Check)

@bot-gradle test this

I've triggered the following builds for you. Click here to see all build failures.

Pull Request Feedback (Trigger) (Check)


raft: make tick unblock and fix potential live lock

/cc @heyitsanthony @gyuho

I have another pr to solve the issue that raft takes long to prepare ready struct. But raft node should be OK even if preparation takes long.

Awesome. Thanks for fixing this. Lgtm.
Sincerely,
Gyu-Ho Lee
On Jun 16, 2016 07:53, "Xiang Li"<EMAIL_ADDRESS>wrote:

I have another pr to solve the issue that raft takes long to prepare ready
struct. But raft node should be OK even if preparation takes long.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/coreos/etcd/pull/5692#issuecomment-226509580, or mute
the thread
https://github.com/notifications/unsubscribe/AGe_ckqMfK-vwkgf75J6Ys5ra-zPdZIkks5qMWNvgaJpZM4I3dzu
.

@gyuho Can you try to redeploy the cluster with the patch? I want to make sure it does fix the problem.

Sure. I will try this PR in a separate cluster.
Sincerely,
Gyu-Ho Lee
On Jun 16, 2016 07:56, "Xiang Li"<EMAIL_ADDRESS>wrote:

@gyuho https://github.com/gyuho Can you try to redeploy the cluster
with the patch? I want to make sure it does fix the problem.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/coreos/etcd/pull/5692#issuecomment-226510580, or mute
the thread
https://github.com/notifications/unsubscribe/AGe_cvzFh55Q1fviUf3mKFpMWY8BKgEoks5qMWQqgaJpZM4I3dzu
.

lgtm

@xiang90 @heyitsanthony Been running this patch in 3 etcd clusters. Over 300 rounds have been running with the live-lock issuem, so this should fix https://github.com/coreos/etcd/issues/5679.
2 of 3 clusters were running with 8-CPU.


Fix MPC input bug for vastly improved steering

This PR is a simple bug fix to a long-standing lateral control problem in Openpilot.  It's not clear why the current logic uses the desired delta as the cur_state input to the MPC, but it's clearly better to use the current delta instead.  This greatly reduces overreaction by the MPC, which results in a high precision execution.
The only negative to this change is that it does cause the PID tuning values to be somewhat inadequate, and steering response is reduced.  However, by increasing steerKpV and steerKiV by 33% and reducing steerRateCost from 0.5 to 0.1, steering response was restored.  I have not changed any tuning values in this PR.
Below is a side-by-side comparison showing identical tests on the same stretch of road at the same speed.  The left side shows the baseline test using the standard value for cur_state delta, and the right side shows the result from this PR.

Hmm, this is now a one line change. @pd0wm does it make sense?

Here is a sample from a remote tuning session on a RAV4.  I suspect Toyota's will benefit the most from this fix.

I deleted an erroneous comment about RAV4 data a couple hours ago after realizing that it was NOT from a RAV4, nor was it from this branch.
However, here is a capture from a RAV4 after some tuning with this fix.

I'm not sure if this is due to improvements in 0.5.13, or due to this PR, but I noticed that my learned steerRatio has gone down significantly.  I believe it has always been at least 0.5 degrees higher than that in the past.  The value for angleOffsetAverage looks the same, which I believe is correct, but I don't recall what stiffnessFactor was in the past.
{   "steerRatio": 16.217367383339074,    "carVin": "",    "carFingerprint": "HONDA ACCORD 2018 SPORT 2T",    "stiffnessFactor": 0.6244330109763914,    "angleOffsetAverage": 0.9613074505751787 }

If we had a perfect model of the system, it would make sense to feedback the actual state to mpc. However, this is not the case (and will never be): we need to let the PID/INDI controller to take care of closing the loop, while MPC remains purely a trajectory generator and should not strongly depend on the system's dynamic response.
In other words, think of:

MPC: kinematic trajectory generator. Does not deal with specific steering system. Unique tuning for all cars. It's a planner.
PID/INDI: dynamic closed loop controller (F = m*a is used here). Deals with the specific car's steerting system. Each car can have its own tuning. It's a controller.

Also, I can think of cases where this change would not work at all. Assume the EPS has a torque deadzone (does not respond until torque cmd exceed a certain value): how would you have the steering to move? With this change, the feedback to MPC would remain the same.

Could this be determined by a new steering or planning type?  I understand that there may be some vehicles that respond differently from the few cars I've tested this with, and it would be a significant effort to retune every car.  However, it seems that the introduction of INDI creates an opportunity to greatly improve lateral performance for the vehicles that currently perform poorly with the existing logic.
Otherwise, it seems that my prior PR for deadzone in pathplanner should be evaluated.

On a side note, I am working on a branch with remote tuning for INDI, which I think would greatly simplify the migration.  I've already moved my 2018 Accord over to INDI with surprisingly little tuning, and am very impressed with the performance.

After thinking more about the comment from @rbiasini, it seems that there is a significant overlap in "roles" between pathplanner and latcontrol.  It seems that pathplanner is using desired angle to communicate torque, and latcontrol is converting that into a torque request with some PID calculations.  Forthermore, it seems that the current logic treats path error as torque error AND latcontrol's PID logic does the same.  I think this is a fundamental flaw, especially when the pathplan is updated at a different frequency AND neither process is considering the actual steer rate reported by the EPS.
It seems that the correct approach would be to output a desired angle (as it does now) that is intended to be the primary control by "angle type" systems, and a desired rate that is primary control for "torque type" systems.  Desired angle should only be used as a secondary measure for torque type systems.
If torque type steering systems were using rate-based feedforwared the bias / offset issues would be eliminated.    Here: https://github.com/commaai/openpilot/pull/593

It seems that pathplanner is using desired angle to communicate torque, and latcontrol is converting that into a torque request with some PID calculations

Not really. pathplanner communicates angle. Angle to torque is done is done by latcontrol alone, which can either be PID or INDI.

Forthermore, it seems that the current logic treats path error as torque error
Not sure what you mean: the only way to go back into the desired path is to apply some torque, which is what the low level controller (PID or INDI) does. Moreover, path error is used by the vehicle model parameter estimator to fix the errors in model parameters.

neither process is considering the actual steer rate reported by the EPS
INDI controller considers the steer rate directly. PID (or any other closed loop linear controller... feel free to put as many poles and zeros as you want) could also do the same.

t seems that the correct approach would be to output a desired angle (as it does now) that is intended to be the primary control by "angle type" systems, and a desired rate that is primary control for "torque type" systems.
I'm all in favor for a steer-rate based feedforward to add to the existing angle based feedforward. They are both true things of the torque that needs to be applied in order to move steering wheel and keep it at an angle, respectively. Let me re-read the PR you mentioned.

Not really. pathplanner communicates angle. Angle to torque is done is done by latcontrol alone, which can either be PID or INDI.

I agree that pathplanner is communicating angle, but that angle is apparently derived using the input delta + new desired rate.  I recall a conversation with @pd0wm about mpc interpolation, and he indicated that the MPC is rate based.
After more specific testing using PID and INDI to compare the results with standard pathplanner as well as with the proposed fixes.  Both forms of control exhibited the same behavior with respect to desired angle.  The desired angle becomes exaggerated with standard logic, which results in more noise in the actual steering angle.
Here is a side-by-side comparison using my 2018 Honda Accord with PID.  Take note of the magnitude of the angle error graph at the bottom for each.  Also note that I have to provide steering assistance several times, since this road requires more steering torque than my Accord will provide.

Here is the same with INDI

Just wanted to report that this "hack" allows me to use the instant angle offset, which dramatically helps center the car and make up for any vehicle model issues. I'm running INDI and my car is now perfectly centered, even on roads with excessive camber. I don't quite understand why feeding MPC the actual state of the vehicle is a no-no in MPC control theory in this application, but I haven't noticed any negative effects.

@zorrobyte  If you had the pleasure spending 1000+ hours writing exotic compensations for this bug, as I have, I think you would not downgrade it with the term "hack".  It's a 2 line "bug fix" that would increase the number of people that use standard OP on a regular basis.

On another note... After some discussion on Discord, I realized that I should provide a more detailed "zoomed in" view of the same long turn using PID with and without this bug fix.  Below is a long left-handed curve that my 2018 Honda Accord can barely make, due to torque limitations.  The graphs make it look like a few broken curves, but it is actually 1 very long curve.
Without this bug fix, the desired angle "ratchets up" way beyond what it really needs, so the steering very slowly approaches the angle that's actually needed.  If the model was perfect, the torque request would taper off smoothly so that the car would find the center of the lane and stay there.  However, this bug causes the car to eventually cross the center of the lane, which triggers the MPC to reverse the steer rate the other way.  This causes the torque to cut out, resulting in a huge over-correction that virtually blows the corner.  This happens often on long curves, and I often have to grab the steering wheel to prevent running into the car or curb next to me.
On the right-hand side, the same curve is shown WITH this bug fix.  As you can see, the corrections are much less aggressive, but still exist.  I think that some work is still needed to prevent the over-corrections that are still occurring, but this is the best I've ever experienced without exotic compensations.

Something I just noticed while scrolling up to look at the other visual aids is that this bug fix allows integration to be more effective.  It seems that preventing the proportional error from being exaggerated just makes everything work better.

Can you share the drive + time for the plots on the right? I'd like to plot the MPC trajectories.

Here is the drive containing all of the data used for the 3 screen shots posted yesterday.  The times in cabana should match what's at the bottom of the graphs, assuming the timezone stays the same in Cabana.
https://my.comma.ai/cabana/?route=ddd3e089e7bbe0fc|2019-06-18--11-40-25&max=106&url=https%3A%2F%2Fchffrprivate-vzn.azureedge.net%2Fchffrprivate3%2Fv2%2Fddd3e089e7bbe0fc%2Fcc5d305a596193d9b1b262f86e19dda1_2019-06-18--11-40-25

Not sure if this would help, but here is the same long curve from the last screen plot yesterday, which shows standard on the left (from yesterday), and my fully customized branch on the right.  This was the only time I was able to make the long curve without assisting the steering.  It also achieved higher peak steering angles, and most of the steering request consisted of feed-forward.
Here is the cabana drive.
https://my.comma.ai/cabana/?route=ddd3e089e7bbe0fc|2019-06-19--10-51-40&max=72&url=https%3A%2F%2Fchffrprivate-vzn.azureedge.net%2Fchffrprivate3%2Fv2%2Fddd3e089e7bbe0fc%2F47f369205ac24098d588ee90a4389fd9_2019-06-19--10-51-40

When you plot the desired MPC trajectory on the same plot it is very clear what happens:

On this plot the trajectory as calculated on t = 5137.6 is plotted on the future states.
What happens is to make this work you had to decrease steerRateCost. This allows the plan to change the angle a lot within one timestep, about 2.5 degrees in 0.05 s. However this plan is not physical and has nothing to what will actually be executed downstream.

I don't really follow your point.  Are you saying that you understand why it works so well, or that it's not possible that it works so well?

After reading your last paragraph again, it seems that you haven't reviewed my data.  My vehicle follows the desired angle MUCH more closely with this fix.  It also does this using much less proportional error.  If you saw my last update using my fully customized logic, you'll see that the torque request itself actually has a strong resemblance to the requested angle AND executed angle, with "feed forward" being the majority of the requested torque value.
Without this fix, there is very little resemblance between the torque request, desired angle, or executed angle, and the majority of the torque request is proportional error.  Is your position that it's more ideal to rely mostly on proportional error to steer the car?

If I'm understanding the diagram that @pd0wm shared, I can see how this change makes the projected path invalid.  From a data processing and model training standpoint, I can see that being an issue.  However, from the controls point of view, the only thing that matters is the first desired angle.  The 19 desired angles that come after that first angle are not used for controls.
Here is a rough sketch of what I think would occur with existing logic (on the left) when the actual angle doesn't execute fast enough to achieve the desired angle vs the proposed logic (on the right).  The first desired angle in each MPC update is what controls would try to execute, which are contained in the red boxes.
I think it's pretty easy to see how the current logic would be problematic for low-response actuators, since the "stacked" plans essentially create a tsunami of missed execution.  That also explains why Toyota's use such high steer rate costs with standard logic, so that the tsunami is less destructive.

@pd0wm  Would you explain the difference between adding 0.05 seconds to the steerActuatorDelay vs using the next delta in the array?  Might a better approach be to use a later element as the desired angle for controls, instead of reducing steerRateCost?

Yes, your diagram is correct. Even though the 19 other timesteps might not be directly used for control, the whole horizon is taken into account for the optimization problem in MPC. If the projected plan from MPC is not actually exectutable this optimization doesn't make sense anymore.
The 0th timestep in MPC should be the point from where you can output any control action. If you would use the delay you are missing one timestep of control action.
If this change makes driving better we should try to figure out what is better about the angles that are outputted. Changing the starting state is not the solution.
One thing I see happening is that once steering angles become larger, the error between desired and actual angle becomes bigger. For example, we could put a higher cost on changing the angle when the angle is already relatively large.

I agree that the angles would grow when the actuator delay is preventing actual execution.  However, for actuators that have more than 0.05 second latency, it seems better for that to occur.
If I had a Toyota myself, I would test it before posting about it, but my thought is that it might be more effective to reduce steerRateCost to a reasonable value, like 0.5, but use an interpolated target angle further down the desired path as the desired angle for controls.  It seems that would create a much more achievable target.
If that approach would be more desirable for you, I'll pursue it.  However, the cur_states[0].delta would still need to be set to the actual angle to prevent the tsunami.

@pd0wm Did you see my question about the difference between using a later delta vs a larger steerActuatorDelay?

The tsunami isn't as bad as you think. It's not just growing all the time. If you share the drive for the left plot I can plot the actual values. I'll also plot multiple plans, so we don't have to guess.
The Prius is already down to 0.5. The better low-level controls is, the lower the steerRate cost can be. But as long as the plan is still moving faster than the actuator can move, we can't do that.
We can't choose a later delta, because MPC will think the control actions it computes between t=0 and t=actuatorDelay will actually be executed, but they won't.

Good to know!
The drive data for the left-hand plots is also contained in the first cabana drive.  I hoped Cabana would have split them up, but it didn't.  You just need to look at the timestamps at the bottom of the graphs, and go there in the drive.

Looking at the paths you shared, it looks like using any of the first few indexes would have the effect of increasing response.  The actuator wouldn't actually reach those angles before the next MPC update, but it would be well on its way.

This is what the path looks like in the original situation

Yeah, that looks like what I was thinking.  However, it looks like there's about 10 degrees of error (20 vs 10), which blocks integration and doesn't unwind well as paths converge.
I set steerRateCost back to standard value of 0.5 (all other costs are standard), and extended the MPC interpolation by 0.1 seconds (between 2nd and 3rd delta).  This worked very well, especially for unwind.  I don't think my car has ever done this curve as well as.
The dark blue line is the interpolated (and smoothed) target angle that is used for PID.  The cabana drive still seems to be processing, but here is the link.
https://my.comma.ai/cabana/?route=ddd3e089e7bbe0fc|2019-06-20--16-32-00&max=25&url=https%3A%2F%2Fchffrprivate-vzn.azureedge.net%2Fchffrprivate3%2Fv2%2Fddd3e089e7bbe0fc%2F476fa32df3c9ed3acd3a95924cd70118_2019-06-20--16-32-00

I was also surprised at how much it improved straight line.

Unfortunately, I must change my opinion about this modification. With good steering sensor data (https://github.com/zorrobyte/betterToyotaAngleSensorForOP) on stock OP, I cannot get the car to steer enough to center, regardless of my gains or costs.
I can set absurdly high values, like 20 outer/innerloop gains, 0.0001 actuator effectiveness, 0 time constant with INDI, or Kp 2.0, Ki 0.5 with PID; MPC costs (path, heading, rate) from stock values, to all 3, to all 0.1, to all 0 and can still not get the car to oversteer (which results in a floaty feeling around center).

The data looks nice, check out the delta graphs. I think that this hack can be useful for vehicles with 0.1 degree precise (and noisy) steering data to prevent oversteering (non-zero steady state). However, at least with good steering angle data, I have worse results than with stock OP and my home-brew angle sensor.
@Gernby I think you should collect data from Tesla and Hyundai (or a TSS2 Toyota car with good steering angle data) to reach a more concrete understanding of the issue scope and if this were to be merged, it be optional per vehicle in CP.

@zorrobyte  Have you been able to achieve better results with a "more standard" OP branch?  You mentioned many things you changed that would definitely not be standard, including the steering sensor itself.

I had the most impressive OP experience I've ever had, and the drive just finished uploading.
After the discussion in this PR, I was able to optimize my approach for MPC interpolation and dampening, and the result is a nearly ideal execution.  I can't imagine it being better.
I suggest reviewing everything in this drive after t=660, since that was my last "tuning" change.
https://my.comma.ai/cabana/?route=ddd3e089e7bbe0fc|2019-06-20--19-43-55&max=35&url=https%3A%2F%2Fchffrprivate-vzn.azureedge.net%2Fchffrprivate3%2Fv2%2Fddd3e089e7bbe0fc%2F794512893a59447d4820d9ab51c3a306_2019-06-20--19-43-55
Here is an update from the "big turn" I've been showing above.  It's almost perfect.

@Gernby I would argue that while my vehicle is modified from factory, my enhancements such as the solid cog and the steer rate sensor have already been implemented in stock form on other models such as TSS2 cars (Steering Angle Sensor), Solid EPS Damper (Camry, EPS on steering rack, no damper).
My experiences are certainly skewed in relation to assessing performance from a typical Prius user, but shouldn't be discounted wholesale. If anything, my vehicle is more indicative of an ideal platform for Openpilot where closer to real-world results may be tested.
I look forward to experiencing your latest work!

@zorrobyte I'm not discounting your opinion.  I'm just asking if you have compared the PR branch against standard OP?  It's a 2 line fix, which is to improve pathplanner (with minor tuning).
I haven't followed much discussion about INDI, but my understanding is that it wasn't possible to completely eliminate oscillation.  I had the same result when I tested INDI, especially at different speeds.  I found that I could tune it really well for 1 speed, and it would only perform well at that speed.  I later found that speed doesn't seem to be used at all for INDI's torque calculation.  I suspect that will still be the case with these fixes in pathplanner.

The comments from @pd0wm and @rbiasini were very helpful in refining my other planning enhancements.  I'm not sure whether the term is appropriate, but I've added what I call "oversampling" to pathplanner, which has provided another significant improvement.
Since I'm using the poly data directly in latcontrol for lane centering (bypassing the MPC), that allows me to ignore the noisy desired angles at the beginning of the path plan, and focus entirely on the important part of the path further down the road.  This eliminates the need to reduce the steerRateCost.  It also allows me to use multiple overlapping path plans to further reduce noise caused by surface irregularities and shadows.
Here are updated side-by-side snapshots showing the results from standard 0.5.13 from last week on the left, and the new results on the right.

Here is the cabana drive for the graphs on the right.  The timestamp was about t=4300.  The graph on the left is the same as the graph last week, which also had a posted cabana drive.

Here is the cabana drive for the graphs on the right.  The timestamp was about t=2550.  The graph on the left is the same as the graph last week, which also had a posted cabana drive.

This is still a big problem in 0.6

Extremely interesting.

It's clear that 0.6.2 is significantly better about exaggerating angle error, but this bug is still a significant issue.
Here is the cabana link from standard 0.6.2
https://my.comma.ai/cabana/?route=ddd3e089e7bbe0fc|2019-07-31--11-36-06&max=26&url=https%3A%2F%2Fchffrprivate-vzn.azureedge.net%2Fchffrprivate3%2Fv2%2Fddd3e089e7bbe0fc%2Fed63e4907cbd31a792c635fbf37127b2_2019-07-31--11-36-06
Here is from my custom branch
https://my.comma.ai/cabana/?route=ddd3e089e7bbe0fc|2019-07-31--16-49-52&max=38&url=https%3A%2F%2Fchffrprivate-vzn.azureedge.net%2Fchffrprivate3%2Fv2%2Fddd3e089e7bbe0fc%2F4af6629763507e09456822b55bb07c24_2019-07-31--16-49-52

Here is a video of the benchmark test from my custom branch earlier today.

I have pushed a better fix for this issue after discovering that cars with higher steering torque (like Toyota) can suffer from the opposite problem than lower torque (like Honda).  With the logic in the original PR, that could cause occasionally cause the steering system to start oscillating.  However, with the improved logic, this is prevented by always using the value that is closest to the prior desired angle as the current delta for the MPC update.

@Gernby I'll give it a try - I've always liked this fix. Maybe we'll see something like this with the MPC refactor coming up

I've given this tweak 24 miles of mixed city, interstate and state highway driving and the change is difficult to notice, which is a good thing. Some roughness has been reduced, or so it seems.
Some A/B testing, or metrics will be needed to validate this change without cognitive bias.


,kkkkkkkkgo;m -jh

Right on, right on.

Do you have a code example for this? Which version did you use?

:) Everything's is good. If you have any questions feel free to ask :) Thank you for looking into glamor ❤

Sorry,  this issues opened by my cat, It likes to lay on my opened laptop.