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

Remove Flux Support #789

Merged
merged 17 commits into from
Feb 1, 2024
Merged

Conversation

sathvikbhagavan
Copy link
Member

No description provided.

@ChrisRackauckas
Copy link
Member

We should keep a Flux test that shows the auto-translation still works, but internally we won't need any more Flux support beyond that and any other Flux tests beyond a simple translation test can be removed.

@sathvikbhagavan
Copy link
Member Author

I am not sure what auto-translation means here? Is there a tool to convert Flux models to Lux?
NeuralPDE handles Flux and Lux separately with a lot of if-else statements. I basically removed those in this PR and kept the Lux part.

@ChrisRackauckas
Copy link
Member

Is there a tool to convert Flux models to Lux?

Yes. Lux.transform. #783 (comment)

@sathvikbhagavan sathvikbhagavan force-pushed the sb/remove_flux branch 2 times, most recently from bb60257 to 8abaf99 Compare January 31, 2024 19:35
@sathvikbhagavan
Copy link
Member Author

@sathvikbhagavan sathvikbhagavan marked this pull request as ready for review February 1, 2024 17:34
@ChrisRackauckas
Copy link
Member

I haven't touched NNRODE and have commented out the include statement of the file because I saw their tests were also commented out - https://github.com/sathvikbhagavan/NeuralPDE.jl/blob/sb/remove_flux/test/runtests.jl#L47. Should I revive it?

No, that needs a lot more work. Keep it separate.

@ChrisRackauckas ChrisRackauckas merged commit e45047d into SciML:master Feb 1, 2024
9 of 11 checks passed
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

Successfully merging this pull request may close these issues.

2 participants