search

desihub / fiberassign

Fiber assignment code for DESI
BSD 3-Clause "New" or "Revised" License
7 stars 8 forks source link

Populate EBV=0 rows in FIBERASSIGN with correct values #414

Closed araichoor closed 2 years ago

araichoor commented 2 years ago

This PR addresses https://github.com/desihub/fiberassign/issues/413. Currently, EBV=0 in the FIBERASSIGN extension of fiberassign-TILEID.fits.gz files for: sky fibers, stand-alone secondary fibers ToO fibers, unassigned fibers. This complicates the downstream analysis. Typically, there will be ~800 fibers with EBV=0 (~700 skys+unassigned, ~100 secondaries); though could be more for tiles with more passes, as more secondaries will be assigned.

We now populate in assign.merge_results_tile() the EBV=0 rows with correct values. This queries the EBV maps in $DUST_DIR. I ve thus also added $DUST_DIR in the list of environmental variables to be defined.

On the NERSC logging node, this takes ~1s. @schlafly: could you test that at KPNO? hoping that $DUST_DIR is already defined, and that the dust maps are there.

Lastly: this adds an astropy dependence in assign.py, I don t know if that matters...

schlafly commented 2 years ago

It's hard for me to test all the way into the real environment without a tag. But I did enter the datasystems environment and check that I could instantiate an SFDMap and query it at a lot of positions, which I think is the only real risky part? Fine from my perspective to merge, and we can do real tests at KPNO from fiberassign/master before cutting the next tag, okay?

araichoor commented 2 years ago

Thanks for checking: if you succeed in executing SFDMap, I guess it should be ok. For what is worth, what I had in mind was to check that in the environment used here: https://github.com/desihub/desisurvey/blob/4eceb0b67968061ec66fe470c83d1cc3101f6cf0/bin/fba-main-onthefly.sh#L30-L47 the $DUST_DIR is defined, and the dust maps are in $DUST_DIR.

Shall I merge to master now?

schlafly commented 2 years ago

Yup, I checked that SFDMap(...).ebv(...) worked in precisely that environment. I'm okay with your merging to master whenever you're ready. I don't think we plan to make a new tag or change the tag used in operations especially soon, though, right---that will come when we've decided what to do about the STD ~bug.

araichoor commented 2 years ago

Ok, I ll thus merge this PR in few mins.