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

Add callable parameter based interface #56

Merged
merged 6 commits into from
Apr 3, 2025
Merged

Conversation

SebastianM-C
Copy link
Collaborator

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

Implements #54 (comment) and makes the lux model a callable parameter in the NeuralNetworkBlock (as mentioned in #51 (comment))

Needs JuliaSymbolics/Symbolics.jl#1508 (for 774aba9 only. If this is an issue I can split this PR in 2).

@ChrisRackauckas Is the new interface okay?
I might need a bit of help on how to write the parameters more generically, since right now the names are hardcoded.
I suspect I can't use the macro form.

I was also thinking of having get_network work on an ODESystem and retrieve the neural network in the NeuralNetworkBlock case, but I'm not sure how can I check that the ODESystem is a NeuralNetworkBlock. We could just assume it is and just reach out for the expected name and hope for the best 😅

@SebastianM-C SebastianM-C force-pushed the smc/refactor branch 2 times, most recently from b2f857a to 2541377 Compare April 2, 2025 11:28
@SebastianM-C
Copy link
Collaborator Author

I fixed the hardcoding of the symbolic variable names and tests pass on the latest release (and fail on pre due to a lot of upstream issues).

@ChrisRackauckas ChrisRackauckas merged commit c847652 into main Apr 3, 2025
9 of 16 checks passed
@ChrisRackauckas ChrisRackauckas deleted the smc/refactor branch April 3, 2025 23:22
@ChrisRackauckas
Copy link
Member

This was missing docs. We should document this.

@SebastianM-C
Copy link
Collaborator Author

Ah, yes, I added the docstring but I didn't add it in the actual doc pages 😅

@ChrisRackauckas
Copy link
Member

Also a quick tutorial.

@TorkelE
Copy link
Member

TorkelE commented Apr 4, 2025

Looks cool! Once there are some docs I will look at changing my Catalyst implementation to use this instead.

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.

3 participants