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

possible inconsistency of healpix values #885

Open
iprafols opened this issue May 4, 2022 · 2 comments
Open

possible inconsistency of healpix values #885

iprafols opened this issue May 4, 2022 · 2 comments

Comments

@iprafols
Copy link
Collaborator

iprafols commented May 4, 2022

Right now both AstronomicalObject and DesiQuasarCatalogue compute the HEALPix of each line-of-sight but not necessarily using the same in_nside.

Some details:

  • The HEALPix in AstronomicalObject is used to save the data in "HEALPix files", i.e. all the deltas within the same HEALPix are saved on the same file. Here in_nside=16
  • The HEALPix in DesiQuasarCatalogue is used to read the data from the spectra file structure. For data in_nside=64 and for mocks in_nside=16.
  • The HEALPix is not computed in DrqQuasarCatalogue
  • In some cases, HEALPix is computed twice for every object
  • This difference in in_nside makes it more complicated to locate objects and could potentially lead to bugs in the future

To fix this I suggest we do:

  1. Move the method add_healpix from DesiQuasarCatalogue to QuasarCatalogue and call the function also from DrqQuasarCatalogue
  2. Pass the HEALPix number to the AstronomicalObject constructor
  3. Update changes to the file structure of DESI outputs to meet the new structure. Check that this is the only thing we are changing.

The consequences of this are:

  • Small speed increase for DESI runs
  • File structure for DESI outputs will be changed, but it will be closer to the input structure
@iprafols
Copy link
Collaborator Author

iprafols commented May 4, 2022

Edit: The HEALPix value in AstronomicalObject is overwritten by method find_nside from Data which is run at the end of Survey.read_data

@Waelthus
Copy link
Contributor

Waelthus commented May 4, 2022

I don't think that this is a bug except for maybe that it isn't done for DRQ, but it might just not be needed there as files are not stored in healpix order anyway (at least from my limited (e)BOSS understanding). The DESI files are just stored in some way that has been decided on at some point, so I guess we should probably just stick to that.

But for the outputs it can be useful to have coarser or finer healpix cells depending on how much data actually went into the analysis, the find_nside iirc just tried to get each file a certain number of objects that were found to be good from previous analyses.

Not sure if the correlation function analysis is using that "optimized" n_side in any way or if you just set a desired one as well there, if the former a change of this output format might have implications down the line which are not necessarily simple.

Just my 5c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants