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

Refactor integrators.py to make it more general. #589

Merged
merged 7 commits into from
Nov 27, 2023

Conversation

junpenglao
Copy link
Member

@junpenglao junpenglao commented Nov 19, 2023

Also add momentum update based on Esh dynamics

Close #587

Also add momentum update based on Esh dynamics

Co-authored-by: Reuben Cohn-Gordon <reubenharry@gmail.com>
@junpenglao junpenglao mentioned this pull request Nov 19, 2023
15 tasks
Copy link

codecov bot commented Nov 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (16df9a6) 99.14% compared to head (8894248) 99.15%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #589      +/-   ##
==========================================
+ Coverage   99.14%   99.15%   +0.01%     
==========================================
  Files          50       50              
  Lines        2215     2252      +37     
==========================================
+ Hits         2196     2233      +37     
  Misses         19       19              

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

junpenglao and others added 3 commits November 20, 2023 17:41
Also add test for esh momentum update.

Co-authored-by: Reuben Cohn-Gordon <reubenharry@gmail.com>
@reubenharry
Copy link
Contributor

I'm getting changed results (on a sampling run) when I substitute in the non euclidean mclachlan instead of the previous minimal_norm integrator. Have you confirmed that the two implementations yield the same results on the same inputs?

@junpenglao
Copy link
Member Author

Yes the two implementation returns identical result: https://colab.research.google.com/drive/1HMnkMM8PNIaaaSAmZyKrZByCd-GKErsN

But to properly test it we still need to add some test:

  • an isotropic Gaussian log_density so we could compare the analytic solution with the integrator.
  • testing energy preservation (and for the Gaussian case, angular momentum preservation) as invariants under the integrator

junpenglao and others added 2 commits November 26, 2023 21:37
Co-authored-by: Reuben Cohn-Gordon <reubenharry@gmail.com>
@junpenglao junpenglao merged commit e0f107f into main Nov 27, 2023
7 checks passed
@junpenglao junpenglao deleted the integrator_refactor branch November 27, 2023 06:03
junpenglao added a commit that referenced this pull request Mar 12, 2024
* Refactor integrators.py to make it more general.
Also add momentum update based on Esh dynamics

Co-authored-by: Reuben Cohn-Gordon <reubenharry@gmail.com>

* Refactor to use integrator generation functions

* Additional refactoring

Also add test for esh momentum update.

Co-authored-by: Reuben Cohn-Gordon <reubenharry@gmail.com>

* Minor clean up.

* Use standard JAX ops

* Adding a test for energy preservation.

Co-authored-by: Reuben Cohn-Gordon <reubenharry@gmail.com>

* fix formatting

---------

Co-authored-by: Reuben Cohn-Gordon <reubenharry@gmail.com>
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.

Generalizing integrators
2 participants