Issue/569/sigmac redundancy

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

Issue/569/sigmac redundancy #582

Closed combet closed 1 year ago

combet commented 1 year ago

This removes redundant functions that called sigma_crit, keeping only the method eval_sigma_crit from the cosmology class.

made changes throughout
removed now unnecessary tests to those redundant functions of the modeling class + modeling functional interface
add sigma_crit test to test_cosmology_parent

combet commented 1 year ago

I cannot figure out what is going wrong with the NC backend, for the new eval_sigma_crit test I added into test_cosmology_parent. I basically moved that test from test_theory and haven't figured out what the matter is. Notebooks are happily running... Any help would be much appreciated @m-aguena, @hsinfan1996?

coveralls commented 1 year ago

Coverage: 100.0%. Remained the same when pulling 01b2e6a1f9bedbee202124927f09e5feae09e8ea on issue/569/sigmac_redundancy into bf74fe9566b3221b959372089907e5a8531b44d4 on main.

m-aguena commented 1 year ago

I cannot figure out what is going wrong with the NC backend, for the new eval_sigma_crit test I added into test_cosmology_parent. I basically moved that test from test_theory and haven't figured out what the matter is. Notebooks are happily running... Any help would be much appreciated @m-aguena, @hsinfan1996?

@combet this happened because the initialization of an attribute of NumCosmoCosmology was being done by NumCosmoModeling, which is a bad practice. Good thing your unittest caught it!

combet commented 1 year ago

Ah great, indeed, thanks for solving this @m-aguena!

hsinfan1996 commented 1 year ago

I cannot figure out what is going wrong with the NC backend, for the new eval_sigma_crit test I added into test_cosmology_parent. I basically moved that test from test_theory and haven't figured out what the matter is. Notebooks are happily running... Any help would be much appreciated @m-aguena, @hsinfan1996?

@combet this happened because the initialization of an attribute of NumCosmoCosmology was being done by NumCosmoModeling, which is a bad practice. Good thing your unittest caught it!

I address this issue in PR #496 by moving the initialization to clmm/cosmology/numcosmo.py. The most recent comment in that PR is about this, but I think @m-aguena fixed it here.

combet commented 1 year ago

I cannot figure out what is going wrong with the NC backend, for the new eval_sigma_crit test I added into test_cosmology_parent. I basically moved that test from test_theory and haven't figured out what the matter is. Notebooks are happily running... Any help would be much appreciated @m-aguena, @hsinfan1996?

@combet this happened because the initialization of an attribute of NumCosmoCosmology was being done by NumCosmoModeling, which is a bad practice. Good thing your unittest caught it!

I address this issue in PR #496 by moving the initialization to clmm/cosmology/numcosmo.py. The most recent comment in that PR is about this, but I think @m-aguena fixed it here.

Cool, thank you @hsinfan1996! So it's fixed in both PR.

I think this is ready for review.