LSSTDESC / CLMM

A Python library for performing galaxy cluster mass reconstruction from weak lensing observables
BSD 3-Clause "New" or "Revised" License
24 stars 20 forks source link

Problem with coordinate implementation #635

Open marina-ricci opened 3 months ago

marina-ricci commented 3 months ago

We recently merged a PR to specify the coordinate system for the shear, but it lacks some functionalities. First, it should be explained in details in a notebook, so that this bug would have been found. E.g : showing a profile taken with an incorrect coordinate system and showing all intermediate steps.

At the moment, the keyword is missing from compute_tangential_and_cross_components. You can pass it to the Galaxy Cluster object, but it does not imply that it will be used (this should be avoided).

Also, we might consider adding a simple way to convert between the two coordinate, once compute_tangential_and_cross_components is run (not having to rerun all steps).

(Note : I discovered this while trying to implement in TXPipe and seeing no changes).

caioolivv commented 3 months ago

Hey @marina-ricci, just to be sure I understand what's the problem: GalaxyCluster.compute_tangential_and_cross_components() should have an option to pass in the coordinate system or should it explain that GalaxyCluster.coordinate_system will be used (or a combination of both)?

Also I agree there should be a function to convert between them. Maybe GalaxyCluster.convert_coordinate_system()? In that case, should you call it and then call GalaxyCluster.compute_tangential_and_cross_components() again or just calling GalaxyCluster.convert_coordinate_system() should already compute new tangential and cross components?

Finally, there's the examples/demo_coordinate_system_datasets.ipynb notebook from another PR which shows the problems of using another coordinate system. Is that what you meant? In that case we could reference it in another notebook?

marina-ricci commented 3 months ago

Hi @caioolivv ! I did not see that the examples/demo_coordinate_system_datasets.ipynb notebook was merged to main. I'm not sure it should stay here in it's current form.

These are my suggestions :

I created a branch with a notebook to show the problem (it's not fully documented but should be understandable)