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

BUG: m_min and m_max astropy quantity error #503

Open
Fox-Davidson opened this issue Nov 23, 2021 · 4 comments
Open

BUG: m_min and m_max astropy quantity error #503

Fox-Davidson opened this issue Nov 23, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@Fox-Davidson
Copy link
Collaborator

Fox-Davidson commented Nov 23, 2021

Describe the bug
When working in the skypy\halos module, the maximum and minimum mass (variable names m_min and m_max) to draw from the halo mass function are converted to astropy quantities before any further functions are called.

To Reproduce
Steps to reproduce the behavior:

  1. Code tested using the function "colossus_mf" in skypy\halos\ _colossus.py through a configuration file
  2. "colossus_mf" calls functions "colossus_mf_redshift" and "colossus_mass_sampler"
  3. No issue in "colossus_mf_redshift" despite using "lnmmin = np.log(m_min*cosmology.h)"
  4. Issue in "colossus_mass_sampler" line "m_h0 = np.logspace(np.log10(m_min* h0), np.log10(m_max *h0), resolution)" producing error "TypeError: no implementation found for 'numpy.logspace' on types that implement __ array_function __: [<class 'astropy.units.quantity.Quantity'>]"

Expected behavior
m_min and m_max should be floats not <class 'astropy.units.quantity.Quantity'> and hence not produce the error. This can therefore be temporarily fixed by using "m_min.value" instead.

Desktop (please complete the following information):

  • OS: Windows
  • Version: 10 Pro

Additional context
Adding debug print statements in "colossus_mf" but before "colossus_mf_redshift" is called (first line of function) shows that it is already an Astropy quantity but nothing could be found in the pipeline files that would do this. The only decorator is for the sky area (@units.quantity_input(sky_area=units.sr)) and m_min/m_max are passed as floats in the config file.
halo_test.yml.txt

@Fox-Davidson Fox-Davidson added the bug Something isn't working label Nov 23, 2021
@rrjbca
Copy link
Contributor

rrjbca commented Nov 30, 2021

@katie-davidson I cannot reproduce the error you are seeing without a minimum working example; please provide a simple config file or python script. However, I note that as currently implemented the function colossus_mf expects m_min and m_max to be floats; see the documentation here. If you are trying to pass astropy quantities for these arguments that could be causing the issue.

@Fox-Davidson
Copy link
Collaborator Author

Sorry @rrjbca I have added a config file now. As far as I can work out I pass them in as floats but by the time they get to the colossus_mf function, they are already Astropy units and I don't know when this happens.

@rrjbca
Copy link
Contributor

rrjbca commented Dec 1, 2021

TLDR: replace the relevant lines in your config with:

  m_min: 1.e+9
  m_max: 1.e+12

In YAML 1.1 1e+9 and 1e+12 are not valid scientific notation so these entries in the config file do not get parsed as floats. But according to astropy they are valid scientific notation, so they get parsed as dimensionless astropy quantities. The problem is that the logspace function isn't implemented to take astropy quantities, even when they are dimensionless i.e. no units. The simplest fix is to therefore add the decimal point to the values in your config so that pyyaml parses them as floats. More generally we could consider the following changes to the code:

  • Modify colossus_mf to handle astropy quantities (as well as floats?) for m_min and m_max
  • Modify the grammar used by the SkyPyLoader for floats and/or quantities

Tagging @Lucia-Fonseca and @ntessore for input.

@Fox-Davidson
Copy link
Collaborator Author

Thanks, I added the dots in the config file and removed the .value from the code and it all works now. Does this count as closed or should I leave it open for the further actions mentioned above?

@Lucia-Fonseca Lucia-Fonseca changed the title m_min and m_max astropy quantity error BUG: m_min and m_max astropy quantity error Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants