Fix on bug causing unphysical values in XCF

[HiramHerrera]

This PR adresses the bug found by @andreicuceu causing the XCF to have un physical values (clearly visible at the line of sight in wedge plots) and therefore causing the Vega fitter to fail.

This bug was caused by the previous downsampling method which used the DESI quasar distribution by redshift bin found during SV. This distribution was used in each HEALpix pixel causing each pixel to have different downsampling fractions and therefore generating a bias in the quasar distribution.

This is fixed by introducing a global fraction for downsampling in each redshift bin computed from the raw mocks in preprocessing. These values are stored into an HDU of the dn_dzdM_EDR.fits file.

This is an example of a DESI-Y5 mock XCF computed using this branch, where the values of the XCF along the light of sight are no longer above 0.

[HiramHerrera]

I just updated the --dn_dzdm flag so it has a None value as default (used to reproduce former DESI mocks e.g desi-2.0, and 3 additional choices ("lyacolore", "saclay" and "ohio"). If lyacolore or saclay options are given, QQ then downsamples QSOs by redshift bin so it matches SV distribution using a fraction precomputed for each type of raw mock and then applies a random r-band magnitude based on the SV distribution. If Ohio option is given no downsampling is applied but the r-band magnitude distribution of QSO is still sampled.

[alxogm]

Hi @sbailey we'd like to merge this PR as most tests passes excepts for something that seems unrelated to this PR unless there is another indication from you or someone else.

Thanks

[moustakas]

I'll look at the failing tests soon @alxogm.

[moustakas]

@weaverba137, I looked at the failing coverage test but the fix isn't obvious to me---can you take a quick look?

The error logs point to this documentation, but I don't see in the workflows configuration file where we're using the deprecated Node12 vs Node16-- https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/

[weaverba137]

@moustakas, I think your comment may already be out of date. What I see is that all tests are successful, including coverage, but data are not being transmitted properly to Coveralls. I can take a look at that, but am I missing something else?

[weaverba137]

And I know why data are not being transmitted successfully to Coveralls: this PR is off of a fork, not a branch, and @HiramHerrera does not have permission to access Coveralls, which is an expected security feature.

[moustakas]

That makes sense.

@HiramHerrera, @alxogm--given that we now understand the coverage failure, go ahead and merge this PR (especially if it's time-sensitive), but going forward we should simply @HiramHerrera as a collaborator.

Oh and before merging please add a note to the change log.