-
Notifications
You must be signed in to change notification settings - Fork 12
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
Differentiable cosmo: Cosmology now a full module with optional Phytorch #72
base: main
Are you sure you want to change the base?
Conversation
… module with optional phytorch
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
==========================================
+ Coverage 85.34% 86.25% +0.91%
==========================================
Files 33 36 +3
Lines 1501 1608 +107
==========================================
+ Hits 1281 1387 +106
- Misses 220 221 +1
☔ View full report in Codecov by Sentry. |
All tests pass now that I have removed phytorch from the testing sequence. If people want to test phytorch, they can uncomment the test I wrote in |
Current status: cannot build boost, which is needed to compile phytorch's cosmology. |
Well, that's a wrap! Phytorch is now testable from within Caustic, with some caveats. The only real casualty is testing on windows....unfortunately, there is some problem with compiling C++ as usual on windows, and so I just decided to eliminate the windows-based Github actions. Who uses Windows anyway? Second thing is that I am relying on a couple of small modifications to phytorch's compilation process in order to help gcc find boost inside the github action, so I have forked phytorch. However, I will be syncing that fork regularly, and the changes are in a place where I don't think there will be any issues going forward. Thirdly, I have a file within the forked phytorch's home directory which I move into pytorch to overwrite a pytorch file which requires a small modification, at least until pytorch gets a pull request and fixes the issue. Fourthly, I had to reduce the accuracy of the critical density wrt to astropy to 1e-2, but I think this is due to floating point precision with very small numbers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Just a few questions
Adding windows test that ignores cosmology
All of Connor's requested changes have been implemented! Windows testing is back but without cosmology. |
If I did not break anything regarding Caustic design principles, I think everything is ready to go. I tested creating a lens with the new Phytorch differentiable cosmology and it works, and the distances (especially gradients) are more accurate with Phytorch.