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

fixes desi_zcatalog --fibermap for new format fibermap #723

Merged
merged 3 commits into from Nov 13, 2018
Merged

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Nov 13, 2018

This PR fixes desihub/desisim#448, where desi_zcatalog --fibermap didn't work for the new format fibermap. This implementation works on zbest files with both old and new format fibermap tables, and adds columns RA, DEC, FLUX_G, FLUX_R, FLUX_Z in both cases, even though the old version used to write MAG instead.

Unlike the discussion in desihub/desisim#448 this morning, I actually have tested it this time, using the 18.7 (old) and 18.11 (new) reference runs in /project/projectdirs//desi/datachallenge/reference_runs/ .

@TEtourneau please take a look if you'd like. Otherwise I'll merge tomorrow morning.

@TEtourneau
Copy link

I tested desi_zcatalog --fibermap, and it works.
Thanks for fixing the bug!

Just a practical question, what's the best way to use different branchs in the desi environment ?
To use desisim and desispec, I use this command:
source /project/projectdirs/desi/software/desi_environment.sh master
But I didn't know how to test desi_zcatalog from the branch target_ra, so I just git clone in my home directory desispec and switched to the branch target_ra.

@sbailey
Copy link
Contributor Author

sbailey commented Nov 13, 2018

Thanks for testing @TEtourneau . Here's what I do for testing branches:

#- Use pre-installed master versions of all packages (updated nightly)
source /project/projectdirs/desi/software/desi_environment.sh master

#- Remove desispec/master from my path
module unload desispec

#- Replace it with my copy
cd $SCRATCH/desi/code/desispec
git checkout target_ra
export PYTHONPATH=$(pwd)/py:$PYTHONPATH
export PATH=$(pwd)/bin:$PATH

I've actually wrapped this in my own module file so that I can module swap desispec/sjb to replace the standard version with my own, but it is equivalent to the above.

@sbailey sbailey merged commit b8504a9 into master Nov 13, 2018
@sbailey sbailey deleted the target_ra branch November 13, 2018 22:47
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.

RA_TARGET changed to TARGET_RA
2 participants