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

Compute and report sky location (exposure time calculator) for every tile. #119

Merged
merged 7 commits into from Jun 14, 2018

Conversation

forero
Copy link
Member

@forero forero commented Jun 12, 2018

Solves #95. Everything is done within the python wrapper by running fiberassign_exec twice.
The final result is a tile file with three tables: FIBERASSIGN (5000 fibers), POTENTIAL (5000 fibers) and SKYETC (20 fibers).

@forero forero requested a review from sbailey June 12, 2018 21:56
Copy link
Contributor

@sbailey sbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for implementing this via the python wrapper. The final features are coming into place.

Let's develop a more robust way to track and cleanup the temporary files, however, e.g. using tempfile.mkdtemp (several desispec unittest use this if you want to see an example). The current method of using tmp_*.* runs the risk of leaving extras behind if something crashes, or accidentally deleting a different tmp_* file that the user may have had. For debugging a --nocleanup option + printing the tempdir location could be convenient.

I may have more comments after actually running this tomorrow.

@sbailey
Copy link
Contributor

sbailey commented Jun 13, 2018

The following test command works for master but produces no output for this sky branch. What's missing?

basedir=/project/projectdirs/desi/datachallenge/reference_runs/18.3
fiberassign --mtl $basedir/targets/mtl.fits \
    --sky $basedir/targets/sky.fits \
    --stdstar $basedir/targets/standards-dark.fits \
    --fibstatusfile $basedir/fiberassign/fiberstatus.ecsv \
    --surveytiles $basedir/fiberassign/dark-tiles.txt \
    --outdir $SCRATCH/temp/

And another reason to use something like tempfile.mkdtemp: the user might be running fiberassign in a directory where they don't have write permission and the current branch fails to be able to write its temporary files there.

@forero
Copy link
Member Author

forero commented Jun 13, 2018

@sbailey I have included the tempfile.mkdtemp calls.

The problem you found comes from the fact that the sky.fits file you are using doesn't have a NUMOBS column, so NUMOBS is set to zero when it's read by fiberassign. When I read that file as the target file to be assigned to the sky fibers, then nothing gets assigned and the tile_*.fits file for those sky fibers are not written down. The absence of those files halts the computation of the tile_*.fits with the extended SKYETC table. I will fix that by setting NUMOBS to one if that column is not included in the input file.

@forero
Copy link
Member Author

forero commented Jun 13, 2018

@sbailey Things seem to be working now. Please check again.

@sbailey
Copy link
Contributor

sbailey commented Jun 14, 2018

Changes look good. Thanks. I verified that it works from a directory in which I don't have write permission and that the previously failing example now works. Merging.

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.

None yet

2 participants