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

Make it easier to pass empty state st = (;) #118

Closed
gdalle opened this issue Aug 3, 2022 · 7 comments
Closed

Make it easier to pass empty state st = (;) #118

gdalle opened this issue Aug 3, 2022 · 7 comments

Comments

@gdalle
Copy link
Contributor

gdalle commented Aug 3, 2022

Hi there!
I am writing code with stateless neural networks, and I would like to be able to pass st = (;) (empty NamedTuple) everywhere instead of remembering the state from the Lux.setup funtion.
Judging by the code in layers/basic.jl, it should work with every layer... except Chain because of this line, where st is annotated as a NamedTuple{fields} instead of a NamedTuple. Is there a specific reason for this?

@avik-pal
Copy link
Member

avik-pal commented Aug 3, 2022

Actually, the other ones should be annotated with fields. Also I don't think it will do what you want. It still tries to access the fields of the named tuple, so for an empty namedtuple it should error.

@gdalle
Copy link
Contributor Author

gdalle commented Aug 3, 2022

At least the PR passes the tests 🤷 Since that Chain was incoherent with the rest, I knew it was either all or nothing on the NamedTuple annotation. Just kinda sad it ends up being all instead of nothing 😉

@gdalle
Copy link
Contributor Author

gdalle commented Aug 3, 2022

Do you want a PR that does the change in the reverse direction, and annotates all st arguments with NamedTuple{blabla}? I think that one would be breaking though

@avik-pal
Copy link
Member

avik-pal commented Aug 3, 2022

It's not breaking, apply* are not public API. Also in either case it would error, with stricter types IMO it is easier to debug.

At least the PR passes the tests

Its because the layers use the correct structure. The failure will only occur if we pass an incorrectly structured nt.

@lungd
Copy link
Contributor

lungd commented Aug 3, 2022

Speaking of tests, are the tests setup and working properly?
With that PR basic.jl#268 should have failed?
Or is it because of the CI settings. I mean, why did that PR pass

@avik-pal
Copy link
Member

avik-pal commented Aug 4, 2022

Speaking of tests, are the tests setup and working properly?

That's just an untested code path.

@gdalle
Copy link
Contributor Author

gdalle commented Aug 4, 2022

OK, so let's close this? One day, I'll make a PR to Lux that gets accepted 💪

@avik-pal avik-pal closed this as completed Aug 4, 2022
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

3 participants