Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading type signature of UpdateFn #707

Open
JGameCreation opened this issue Jul 24, 2024 · 4 comments
Open

Misleading type signature of UpdateFn #707

JGameCreation opened this issue Jul 24, 2024 · 4 comments
Assignees

Comments

@JGameCreation
Copy link

As specified in base.py, the UpdateFn should only take in two arguments: an RNG key and the previous state. However many samplers take in additional arguments. For example, sgld also takes a minibatch of data and a step size after the rng and state. Maybe let's add *args to the signature of UpdateFn.__call__?

@albcab
Copy link
Member

albcab commented Jul 24, 2024

not sure if adding *args to UpdateFn.__call__ works or will just force us to add # type: ignore[arg-type] to every use of SamplingAlgorithm (instead of just the ones that deviate from the two arguments you mentioned).

but you can try opening a PR with the change to see if it works @JGameCreation, and we can see from there.

@JGameCreation
Copy link
Author

I created a draft pull request but it doesn't seem to run any tests for some reason

@junpenglao
Copy link
Member

junpenglao commented Jul 25, 2024

I updated the setting so the tests should run for you now.

@gil2rok
Copy link
Contributor

gil2rok commented Aug 13, 2024

Related to #700

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants