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

Quickcat update #518

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Quickcat update #518

wants to merge 11 commits into from

Conversation

rstaten
Copy link
Contributor

@rstaten rstaten commented Dec 13, 2019

This PR:

  • Updates quickcat and quicksurvey to run using current fiber assignment and mtl generation.
  • Updates quickcat to get OII flux for ELGs from updated truth HDU configuration.
  • Adds RA, DEC, TRUEZ, OIIFLUX, FLUX_R to quickcat output zcat.fits file.
  • Allows quicksurvey to propagate observing conditions to quickcat from an input exposures file.
  • Updates quickcat model parameters based on redrock results from svdc-summer2019c.

Notes:

  • Once this is merged, I can update quicksurvey_example to reflect the change from running 'fiberassign' to 'fba_run'.
  • I removed backward compatibility for previous versions of fiber assignment (I can put this capability back in if needed).
  • I changed the QSO_SPLIT parameter to 2.1 from 2.0 based an SV discussion from the Ohio workshop.

@sbailey, @julienguy, Do you have any comments on this, in particular with the backwards compatibility for fiber assignment and updated models?

Here are some example plots showing the updated fits:
elg_fit
lrg_fit
loqso_fit
hiqso_fit

@sbailey
Copy link
Contributor

sbailey commented Dec 17, 2019

Thanks @rstaten . Please check and fix the failing unit tests:

======================================================================
4699ERROR: test_quickcat (desisim.test.test_quickcat.TestQuickCat)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/desihub/desisim/py/desisim/test/test_quickcat.py", line 121, in test_quickcat
    zcat1 = quickcat(self.tilefiles[0:2], self.targets, truth=self.truth, perfect=True)
  File "/home/travis/build/desihub/desisim/py/desisim/quickcat.py", line 592, in quickcat
    fibassign, header = fits.getdata(infile, 'FASSIGN', header=True)
  File "/home/travis/miniconda/envs/test/lib/python3.5/site-packages/astropy/io/fits/convenience.py", line 199, in getdata
    hdu = hdulist[extidx]
  File "/home/travis/miniconda/envs/test/lib/python3.5/site-packages/astropy/io/fits/hdu/hdulist.py", line 301, in __getitem__
    self._positive_index_of(key))
  File "/home/travis/miniconda/envs/test/lib/python3.5/site-packages/astropy/io/fits/hdu/hdulist.py", line 721, in _positive_index_of
    index = self.index_of(key)
  File "/home/travis/miniconda/envs/test/lib/python3.5/site-packages/astropy/io/fits/hdu/hdulist.py", line 702, in index_of
    raise KeyError('Extension {!r} not found.'.format(key))
KeyError: "Extension ('FASSIGN', 1) not found."

It is ok to not be backwards compatible with old fiberassign files, but at least tests should pass.

Could you also provide plots comparing the new efficiencies with the old ones? i.e. are these big changes or small?

@michaelJwilson
Copy link

Hi both,
apologies, but I can't dig into the changes at the minute. One point: we agreed to mimic "the LSS catalogue format" for quickcat during the Ohio meeting. Based on Will's doc., I had a first attempt at creating this from the SV data challenge. Details are below. Ashley has agreed to look at it in early Jan., while I should be able to get back to it in the not-too-distant future.

If nothing else, the quickcat output should be one HDU trying to mimic the LSS catalogue, with a separate HDU for the truth information.

Hi all,
in case of duplicated work, I tried to generate Will's catalogue starting from the SV simulation.
Many more details /work will have to follow, but there was enough to make a decent start. Hopefully,
something to build upon that shows what's out there already and I'll have a chance to come back to it
soon.

The output is here:
https://portal.nersc.gov/project/desi/users/mjwilson/lsscat_v1.fits

(Date-stamped tiles: https://portal.nersc.gov/project/desi/users/mjwilson/dst_tiles.fits)

Script is here:
https://portal.nersc.gov/project/desi/users/mjwilson/lss_cat.py

I kept the origin of a property aswell as the requested form. E.g. the redrock SPECTYPE
[STAR, GALAXY, QSO] needed for the Z_VETO_FLAG. Let me know if you're interested.

Thanks.

Mike

@michaelJwilson
Copy link

michaelJwilson commented Dec 28, 2019

I fixed the unit tests.

Ryan, the fits for redshift efficiency look off. E.g. the Gaussian STD is negative for LRG: I resorted to the old yaml file. Presumably, because it's fitting to ~1.00.

I also added MWS to the z perturbation dict., in lieu of calling STAR and WD independently in the tests.

Also, removed DECAM_FLUXS given incompatibility with actual targeting files, and added fluxes
[nanomaggies] in the right ballpark for LRGs.

I'll look to merge with #501 and the new LSS catalog (desihub/LSS#4 (comment)).

@rstaten
Copy link
Contributor Author

rstaten commented Dec 29, 2019

Hi Michael,

Thanks for the information and fixing the unit tests/other updates. I agree that the LRG parameters seem off, but as you mentioned this is because it is fitting to 1.00. Below are some plots comparing the old vs. new fits. I didn't include QSO comparisons because I changed the cutoff for low vs. high redshift from z=2.0 to z=2.1, but I can make some plots that compare apples to apples if needed.

Let me know if you'd like me to provide any other updates for this branch. If it's better to go ahead and merge this and discuss other changes for future branches that's no problem.

elg_comparison
lrg_comparison

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

Successfully merging this pull request may close these issues.

3 participants