search

LSSTDESC / CCL

DESC Core Cosmology Library: cosmology routines with validated numerical accuracy
BSD 3-Clause "New" or "Revised" License
144 stars 65 forks source link

Update correlation documentation #744

Closed jablazek closed 3 years ago

jablazek commented 4 years ago

The readthedocs documentation for correlation functions (including correlation_3d) is incorrect. It may be just that the function is incorrectly specified (with an extra .correlation in the call), but there may be other issues. We should also update documentation once the added functionality in Issue #743 is included.

aferte commented 4 years ago

Has this been fixed already? I don't really understand the issue.

c-d-leonard commented 4 years ago

So the issue is that @jablazek was finding that pyccl.correlation.correlation_3d (as given here: https://ccl.readthedocs.io/en/v2.0.0/api/pyccl.correlation.html) wasn't working, but pyccl.correlation_3d was. So the thing to do would be to verify that you do actually get an error when trying to do pyccl.correlation.correlation_3d as given in that documentation. I thought this was just a documentation issue but actually I think if you find that this is indeed giving you an error I would like to look into why because it seems like it should be working .. I'm signing off for the day now but happy to help look at this tomorrow.

jablazek commented 4 years ago

Yeah, thanks @c-d-leonard , that sounds familiar. I also don't recall if this is a pure documentation issue or some broken functionality due to other code updates.

aferte commented 4 years ago

Ok, thanks! I just checked: pyccl.correlation.correlation_3d(cosmo,a,r) gives indeed an error (AttributeError: 'function' object has no attribute 'correlation_3d') and pyccl.correlation_3d(cosmo,a,r) seems to be working fine.

aferte commented 4 years ago

Ok - I think that is happening to all the methods in correlation.py

aferte commented 4 years ago

But when I run getattr(pyccl,'correlation_3d'), it says <function pyccl.correlation.correlation_3d(cosmo, a, r)>. Weird

jablazek commented 3 years ago

@c-d-leonard . Been working with @Andromedanita on this. Why do you expect that both function calls would work? Looking at the init in pyccl, I see the following:

from .correlation import ( correlation, correlation_3d, correlation_multipole, correlation_3dRsd, correlation_3dRsd_avgmu, correlation_pi_sigma)

My (admittedly limited) understanding is that this puts all of the imported functions directly in the pyccl namespace. So shouldn't pyccl.correlation_3d() work but pyccl.correlation.correlation_3d() fail?

Assuming you agree, we can just do a quick doc update.

jablazek commented 3 years ago

@EiffL @c-d-leonard : OK! I figured it out. It actually should work either way (e.g. ccl.correlation.corrlelation_3d or ccl.correlation_3d).

But, there is a function in correlation.py just called correlation, which replaces the entire ccl.correlation namespace with a function.

In fact, all the pyccl functions are organized/imported this way. for instance, pyccl.power. But most do not have the name collision.

So, we could fix this by noting that this particular function can only be called the second way. Or we rename the enclosing file correlations.py (or similar).

jablazek commented 3 years ago

I prefer the latter. I think that this change won't break any code, since ccl.correlation.___ is already broken.