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

fiberassign tertiary: which recorded path in header for TOO? #449

Open
araichoor opened this issue Nov 18, 2022 · 5 comments
Open

fiberassign tertiary: which recorded path in header for TOO? #449

araichoor opened this issue Nov 18, 2022 · 5 comments

Comments

@araichoor
Copy link
Contributor

I m trying to make a decision for the design of the tertiary tiles.
posting that here, to have feedback/suggestions (e.g. @schlafly , @moustakas )
and also so that things are documented.

context:

For those tertiary programs, the design process usually is like:

  • generate ToO-PROGNUM-FIRST_TILE.ecsv
  • generate fiberassign-FIRST_TILE.fits.gz
  • then generate ToO-PROGNUM-SCND_TILE.ecsv, with reading assignment from fiberassign-FIRST_TILE.fits.gz
  • then generate fiberassign-SCND_TILE.fits.gz
  • and so on,

so I ve kind of cornered myself here, because I need to work from a local $DESI_SURVEYOPS checkout so that I can write files there.
my bad, when making the PR #437, I missed this interleaved case of ToO*ecsv file creation and fiberassign.
if no action is taken, it s my local checkout path which will be recorded in the fiberassign header.
it s not dramatic, but it s not great either.

I could think of the following workarounds:

workaround 1:
in the header reported paths, use DESISVOP (ie DESI_SURVEYOPS fitting the 8-character limit) as a replacement of the $DESI_SURVEYOPS path.
we currently have something like:

DESIROOT= '/global/cfs/cdirs/desi'                                              
GFA     = 'DESIROOT/target/catalogs/dr9/1.1.1/gfas'                             
MTL     = 'DESIROOT/survey/ops/surveyops/trunk/mtl/main/dark'
TOO     = 'DESIROOT/survey/users/raichoor/svn/surveyops/trunk/tertiary/...'  

it would be replaced by:

DESIROOT= '/global/cfs/cdirs/desi'      
DESISVOP= '/global/cfs/cdirs/desi/users/raichoor/svn/surveyops/trunk'                                        
GFA     = 'DESIROOT/target/catalogs/dr9/1.1.1/gfas'                             
MTL     = 'DESISVOP/mtl/main/dark'
TOO     = 'DESISVOP/tertiary/...'  
  • pros: it kind of achieves the goal, provided that one should/could ignore the actual DESISVOP keyword value and use the "real" /global/cfs/cdirs/desi/survey/ops/surveyops/trunk/.
  • cons: caveat mentioned in the pros; and also, it would make the change for the main tiles.

workaround 2:
once files are generated, I could edit the fiberassign-TILEID.fits.gz header and the fiberassign-TILEID.log file, with replacing my local checkout path with the official $DESI_SURVEYOPS path.
I would add a function like fiberassign.fba_tertiary_io.update_desisvop_path().

  • pros: achieves the goal.
  • cons: not super nice to a posteriori edit files.

workaround 3:
run the fiberassign design with some special privileges which authorize me to write in the "official" $DESI_SURVEYOPS folder.

  • pros: achieves the goal, no code change needed.
  • cons: it may be not recommended to do something like that.

remark:
the workaround I found for the tertiary calibration program only worked because it s just one tile at a time, there; so I can generate the ToO-PROGNUM-FIRST_TILE.ecsv file, commit it to svn, wait for it to be picked up, then run fba_launch.
but I cannot do that when there are lots of tiles in a design.

@schlafly
Copy link
Contributor

Thanks Anand. Klaus says we may be able to observe with 4 spectrographs tomorrow night, so we need to design something quickly. Can we, e.g., run from a pre-PR #437 version of fiberassign? Sorry, that's also somewhat horrible, but...

@araichoor
Copy link
Contributor Author

that s a good suggestion in the meantime, thanks!
fiberassign/5.6.0 should be fine (https://github.com/desihub/fiberassign/blob/main/doc/changes.rst#560-2022-04-22), as we don t add true ToOs to the tertiary targets.

I ll run that from DESI_ROOT/survey/fiberassign (as usual), and the ToO file paths will point there.

@sbailey
Copy link
Contributor

sbailey commented Nov 18, 2022

Regarding tomorrow night, let me remind us of my repeated admonition to not do target selection + fiberassign in a rush because we have typically regretted that in the past. I'd especially encourage signoff from @geordie666 that we are using the targeting infrastructure in the intended way, and @moustakas as the person who has most deeply gone down the rabbit hole of past difficulties in algorithmically reproducing what we did.

My first choice would be something like option 1, but fully spell out $DESI_ROOT and $DESI_SURVEYOPS like

GFA     = '$DESI_ROOT/target/catalogs/dr9/1.1.1/gfas'                             
MTL     = '$DESI_SURVEYOPS/mtl/main/dark'
TOO     = '$DESI_SURVEYOPS/tertiary/...'  

Reasoning: a directory with "raichoor" in the name may disappear in the future and the "official" checkout at "/global/cfs/cdirs/desi/survey/ops/surveyops/trunk" will continue to evolve anyway as more files are added and might even move off of CFS someday (remember /project?). The important point of recording this in the headers is so that we can trace back to the files in the future, and if we define that $DESI_SURVEYOPS means an environment variable that points to a checkout of https://desi.lbl.gov/svn/data/surveyops/trunk, then the important piece is which file in the checkout was used, not where that checkout was on disk.

That option would require new code, and I could understand if @moustakas or others wanted to push back on changing how those header keywords get parsed.

My second option would be something like the remark in option 3:

the workaround I found for the tertiary calibration program only worked because it s just one tile at a time, there; so I can generate the ToO-PROGNUM-FIRST_TILE.ecsv file, commit it to svn, wait for it to be picked up, then run fba_launch.
but I cannot do that when there are lots of tiles in a design.

would that be viable to do if instead of "wait for it to be picked up", you would run "svn update" as the desi user in $DESI_SURVEYOPS as often as you needed so that you can proceed?

I don't like option 2; if our workflow requires us to a posteriori edit files even if we didn't make a mistake, that should be a red flag.

@araichoor
Copy link
Contributor Author

"would that be viable to do if instead of "wait for it to be picked up", you would run "svn update" as the desi user in $DESI_SURVEYOPS as often as you needed so that you can proceed?"
=>
that should work -- though a bit annoying to monitor when there are 25 tiles :)
I can try that for the m31 design tonight, and see how it goes.
note to myself: avoid the cron job at the top of each hour.

about your first choice:

  • we should have chosen that in the very first place! we did something kind of half-way with the DESIROOT;
  • in addition to code change, as I said, note that this also affect the main tiles to come.
    in the spirit of not doing code-change in a rush, I won t do that for this round, but we can keep discuss it, and hopefully implement it at some point.

thanks for the feedback!

@araichoor
Copy link
Contributor Author

"would that be viable to do if instead of "wait for it to be picked up", you would run "svn update" as the desi user in $DESI_SURVEYOPS as often as you needed so that you can proceed?"
=>
I ve been thinking more on this one: for tonight, I think I ll keep things simple, using @schlafly s suggestion to use fiberassign/5.6.0.
original files will live in $DESI_ROOT/survey/fiberassign (as for ~all previous special tiles so far), so it should be fine, even if not as I had wished.

though making those svn up could work, it makes the process quite fragile for several reasons:

  • it makes me commit things before having actually validated everything;
  • it makes the overall process not easily reproducible;
  • again, here, each ToO-PROGNUM-TILEID.ecsv file depends on the fiberassign runs of the previous TILEIDs, so things are interleaved; ie it s not just a matter of running fba_launch.

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

No branches or pull requests

3 participants