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

Differentiable cosmo: Cosmology now a full module with optional Phytorch #72

Open
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

mjyb16
Copy link
Collaborator

@mjyb16 mjyb16 commented Aug 22, 2023

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.

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.91% 🎉

Comparison is base (33cf6eb) 85.34% compared to head (4da6311) 86.25%.
Report is 13 commits behind head on main.

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     
Files Changed Coverage Δ
caustic/cosmology/FlatLambdaCDM.py 100.00% <100.00%> (ø)
caustic/cosmology/LambdaCDM.py 100.00% <100.00%> (ø)
caustic/cosmology/__init__.py 100.00% <100.00%> (ø)
caustic/cosmology/base.py 84.21% <100.00%> (ø)

... and 3 files with indirect coverage changes

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

@mjyb16
Copy link
Collaborator Author

mjyb16 commented Aug 22, 2023

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 test_cosmology.py, but this requires installing phytorch of course. Phytorch has been removed from the init.py inside the cosmology package, so Caustic will not try to import phytorch unless you explicitly import one of the cosmologies that uses it.

@mjyb16 mjyb16 added the enhancement New feature or request label Aug 22, 2023
caustic/lenses/base.py Outdated Show resolved Hide resolved
caustic/cosmology/LambdaCDM.py Show resolved Hide resolved
caustic/cosmology/LambdaCDM.py Show resolved Hide resolved
test/test_cosmology.py Outdated Show resolved Hide resolved
@mjyb16
Copy link
Collaborator Author

mjyb16 commented Aug 25, 2023

Current status: cannot build boost, which is needed to compile phytorch's cosmology.

@mjyb16
Copy link
Collaborator Author

mjyb16 commented Aug 27, 2023

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.

Copy link
Member

@ConnorStoneAstro ConnorStoneAstro left a 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

.github/workflows/python-app.yml Outdated Show resolved Hide resolved
.github/workflows/python-app.yml Show resolved Hide resolved
.github/workflows/python-app.yml Outdated Show resolved Hide resolved
caustic/cosmology/FlatLambdaCDM.py Outdated Show resolved Hide resolved
caustic/cosmology/__init__.py Show resolved Hide resolved
@mjyb16
Copy link
Collaborator Author

mjyb16 commented Aug 31, 2023

All of Connor's requested changes have been implemented! Windows testing is back but without cosmology.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants