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

SciMLCompat + Enzyme #80

Closed
wants to merge 24 commits into from
Closed

Conversation

SCiarella
Copy link
Collaborator

This PR implements a useful interface to use IncompressibleNavierStokes.jl (INS) in the SciML ecosystem.
Secondarily, this PR proposes some extension to INS to make it compatible with Enzyme.jl to perform AD.

At the moment this PR is marked as a draft, since there are a series of points that needs to be addressed.


SciML

We make INS compatible with SciML by approximating the DAE structure of the NS equation by using a projection operator.
It is possible to prove that this projection is consistent with the DAE in the presence of constant or periodic boundaries. Here we show for a test run that the SciML implementation has performance comparable to INS:
plot_4

and that the solution is the same within machine precision
plot_5

while we confirm that (using Float64 precision) the divergence remains $$<10^{-11}$$ for the projected dynamics.


Enzyme

We introduce create_right_hand_side_enzyme by redefining some of the evaluations performed in INS to comply with Enzyme, in particular with respect to temporary variables (that we precompile inside the functions), avoiding unorthodox functions (like KernelAbstractions._get_backend()) in the chain of operation, and enforcing some type stability (in particular avoiding structs and tuples replacing them by ComponentArrays).

In general_Enzyme_benchmark.ipynb we show the speedup that can be achieved by using Enzyme and we confirm the result for a-priori fitting using INS in Enzyme_vs_Zygote_a-priori.jl, by obtaining the following result:

plot_159

Efficiently implementing a-posteriori training with INS+SciML+Enzyme is still a work-in-progress.



TODO

  • implement some tests that numerically compare Enzyme.autodiff to finite difference methods using ChainRules
  • finalize the structure for the a-posteriori fitting with Enzyme. This may require the closure model (a Lux.Chain) to act inside the force rather than the current implementation
  • further tests on the rules defined in src/enzyme.jl to confirm that they do not introduce any numerical error over INS
  • restructuring of the notebooks and examples
  • discuss the possibility of implementing custom Enzyme pullback rules and what consequences this could have on the structure of this PR

@SCiarella SCiarella marked this pull request as draft August 21, 2024 16:07
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.77%. Comparing base (a180524) to head (f11f0d3).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #80   +/-   ##
=======================================
  Coverage   51.77%   51.77%           
=======================================
  Files          32       32           
  Lines        3374     3374           
=======================================
  Hits         1747     1747           
  Misses       1627     1627           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SCiarella
Copy link
Collaborator Author

This issue needs to be addressed in order to be able to write custom pullback rules for Enzyme

@SCiarella
Copy link
Collaborator Author

Not needed anymore after #116

@SCiarella SCiarella closed this Nov 11, 2024
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