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

Issue/635/Modify coordinate system implementation #649

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

caioolivv
Copy link
Collaborator

Recreating PR #636 with correct branch name.

ts485 and others added 15 commits July 19, 2024 08:15
…effects with instructions to use example data (#629)

* feat: Add coordinate_system parameter to compute_tangential_and_cross_components and _compute_lensing_angles functions

* Update documentation to account for new coordinate_system option

* feat: Add coordinate_system conversion test to test_compute_lensing_angles_flatsky function

* Add coordinate_system conversion test to test_compute_lensing_angles_astropy function and fix bug in coordinate system conversion bug in compute_lensing_angles_astropy

* Set coordinate_system option during creation of GalaxyCluster object

* adding a new notebook and data

* test

* new notebook to test coordinate systems and shear measurements

* Removed debuggin print in test_dataops.py

* editing README for the data availability

* moved tomomi's notebook to examples

* added prelude to Tomomi's notebook

* Fixed typo and set default coordinate_system to "pixel"

* Added option to choose coordinate system for generated mock catalog

* Added ellipticity coordinate system conversion tests to test_galaxycluster.py and test_mockdata.py

* Added documentation to other test cases in Tomomi's notebook.  Still need to incorporate Caio's added functionality for sky vs. pixel coordinates in CLMM

* Fixed formatting

* starting to work through and document example of lensing signal calculation of HSCY3 cluster without CLMM, but need to change colossus dependency because it is not easy to install on NERSC

* Finished baseline documentation, including changing colossus cosmology dependency in notebook to the astropy one so this is easier to run on NERSC

* minor change to clarify sections of notebook

* added kwargs to GalaxyCluster instance definition in preparation for re-installing latest CLMM with Caio's added coordinate_system kwargs

* Updated logic for lensing angle conversion between coordinate systems in _compute_lensing_angles_flatsky and updated tests

* Revert "Updated logic for lensing angle conversion between coordinate systems in _compute_lensing_angles_flatsky and updated tests"

This reverts commit 4a1faa6.

* Added coordinate_system validation and extra tests

* Changed example notebook to document new coordinate_system option

* examples/test_coordinate.ipynb has additional minor markdown detail

* test_coordinate.ipynb now runs from top to bottom using Caio's coordinate system implementation!

* adding additional docstrings to Tomomi's from-scratch calculation of lensing from HSC data

* refactoring code into functions to make loop over cluster lenses  more readable and some renaming of variables, e.g. sum to profile_sum, to avoid name space problems

* made additional refactoring changes, but refactored functions to calculate angles between source and lens, etc. likely broke something in the background selection (now empty)

* some of the refactoring incorporated with pre-broken code

* cleaned up the loop over rbins

* In line comments for portions of calculation

* modified reverted commit to when the functions did not break, added in documentation and light refactoring from latest commit

* renamed the notebook for consistency

* docs/doc-config.ini now includes the demo_coordinate_system notebook

* feat: Add coordinate_system parameter to compute_tangential_and_cross_components and _compute_lensing_angles functions (#624)

* feat: Add coordinate_system parameter to compute_tangential_and_cross_components and _compute_lensing_angles functions

* Update documentation to account for new coordinate_system option

* feat: Add coordinate_system conversion test to test_compute_lensing_angles_flatsky function

* Add coordinate_system conversion test to test_compute_lensing_angles_astropy function and fix bug in coordinate system conversion bug in compute_lensing_angles_astropy

* Set coordinate_system option during creation of GalaxyCluster object

* Removed debuggin print in test_dataops.py

* Fixed typo and set default coordinate_system to "pixel"

* Added option to choose coordinate system for generated mock catalog

* Added ellipticity coordinate system conversion tests to test_galaxycluster.py and test_mockdata.py

* Fixed formatting

* Updated logic for lensing angle conversion between coordinate systems in _compute_lensing_angles_flatsky and updated tests

* Revert "Updated logic for lensing angle conversion between coordinate systems in _compute_lensing_angles_flatsky and updated tests"

This reverts commit 4a1faa6.

* Added coordinate_system validation and extra tests

* Changed example notebook to document new coordinate_system option

* Fixed all the documentation for coordinate_system and changed names from 'pixel'/'sky' to 'euclidean'/'celestial'

* Added reference for defitions of coordinate systems

* Fixed bug in coordinate system conversion in mock data generation

* Test ValueError raise for coordinate_system in test_mockdata.py

* Added more explanations about ellipticity coordinate system

* Fixed typo

* uodated tag to 1.12.4

---------

Co-authored-by: Marina Ricci <[email protected]>

* removed a character in a markdown cell that tests complained about in making docs

* swapped pixel/euclidean and sky/celestial in notebook

* reverted two files to try to match main

* removed outputs in the notebook

* removed OTHER notebook outputs

* remove empty cells in notebooks

* made minor edits in response to review, primarily with generalizing file paths and removing comments or redundant text in the markdown

* incorporating original demo_mock_cluster.ipynb from main to remove changes to the file in this branch that should not have been committed

* modified the Oguri example to also show incorrect coordinate assumption as noted by Caio

* got rid of dollar signs inside markdown code snippet env hoping this will get the checks passed

* added review contributions line :-)

* added a note for the CosmoDC2 clusters so the user is aware to not necessarily expect a strong signal in center regions

* removed extra cell at end

* removed rogue savefig

* clarified why there might be less signal in the TXPipe outputted CosmoDC2 thing

* Formatted demo_coordinate_system_datasets.ipynb notebook

---------

Co-authored-by: Caio Lima de Oliveira <[email protected]>
Co-authored-by: Camille Avestruz <[email protected]>
Co-authored-by: Marina Ricci <[email protected]>
@caioolivv caioolivv marked this pull request as draft November 8, 2024 14:48
@caioolivv
Copy link
Collaborator Author

I've noticed I still have to update other notebooks that had coordinate system examples. Working on it right now.

@coveralls
Copy link

Coverage Status

coverage: 100.0%. remained the same
when pulling 08fde7c on issue/635/coordinate_implementation
into cfa7f11 on main.

@marina-ricci
Copy link
Collaborator

@caioolivv is this PR ready for review? If so, could you convert from 'draft' to 'ready to review' ?

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.

4 participants