Tootile

[araichoor]

This PR adds three arguments to fba_launch, to handle the creation of ToO-dedicated tiles:

• targ_std_only: only picks the standard stars from the MTL ledgers (fba_launch_io.create_mtl());
• too_tile: accepts ToO targets for dedicated tiles (fba_launch_io.create_too());
• goaltype: allows one to manually set goaltype (which was previously tied to program).

The default values make those changes backwards-compatible for the Main survey.

Example of invocation:

fba_launch --outdir $DESI_ROOT/users/raichoor/fiberassign-m31/ --tileid 999995 --tilera 11.185 --tiledec 38.768 --survey main --program BACKUP --dtver 2.2.0 --hdr_survey special --hdr_faprgrm m31 --nosteps scnd --goaltime 7200 --sbprof PSF --targ_std_only --too_tile --goaltype DARK

As this M31 tile is outside the DR9 footprint, we need to query the BACKUP program to:

• get the Gaia-based sky lookup locations;
• get the Gaia-based standard stars.

However, it will be observed in dark program, hence:

• the --goaltype DARK argument (to reflect sky background);
• the --targ_std_only argument (to avoid really bright backup targets to be assigned).

[araichoor]

@geordie666 : thanks for agreeing to review this PR!

As said in slack, there are two parts:

• the "original" part which motivated this PR (commits up to bee74f5, included): this enables a special mode in fiberassign, to design ToO-dedicated tiles; most is described in the original PR comment;
• the "added" part -- mostly consisting of adding bin/fba_tertiary_too and py/fiberassign/fba_tertiary_io.py -- which sets in place the code to design the ToO-tertiary-PROGNUM-TILEID.ecsv files, which would then be used by fba_launch as custom ToO tiles; extended description of the approach is here: https://desi.lbl.gov/trac/wiki/SurveyOps/TertiaryPrograms?version=12.

About the "original" part, on fiberassign:
I did check on Mar. 22 that all main tiles could be reproduced with that branch version.
I ve tried since yesterday to rerun that, but cori has been very slow, preventing me to complete that test; though I expect it should not be a problem, as the code touching fba_launch and fba_launch_io.py has not changed since then.

About the "added" part, on creating the ToO-tertiary-PROGNUM-TILEID.ecsv files:
With looking more quietly at that, I m actually wondering if the "added part" shouldn t be in desitarget, as this is "pure" targeting file manipulation, no actual fiber assignment is done (bin/fba_tertiary_too and py/fiberassign/fba_tertiary_io.py); maybe that could be less confusing.
Please let me know what you think about that; I m fine either ways.

[geordie666]

I reviewed the PR for content, although I didn't check whether the code actually runs (under the assumption that Anand must have tried that, already.

I've supplied a handful of comments for consideration.

I think the aspect of the code that makes me most nervous is the potential for a user to provide their preferred bit-names from the desi_mask and scnd_mask. I think we should always default to SCND_ANY for the desi_mask for tertiary targets and always default to one of the TOO bit-names for the scnd_mask. Otherwise we could break the unique correspondence between primary targets and an associated bit-name.

I know that enforcing, e.g., DESI_TARGET=SCND_ANY could make processing downstream a little harder for folks proposing tertiary programs. But, I don't like the thought of, e.g., LRG in the DESI_TARGET column referring to different selections for primary and tertiary programs. It's too easy for a future user to not check on release=8888 in decode_targetid and think that a tertiary target that has bit-name LRG set refers to the exact-same-selection as a primary target with bit-name LRG set.

My preferred method for proposers of tertiary targets to handle "different bits" would be for them to create their own designations/bits in a row-by-row parallel file, to which they could match on TARGETID.

[geordie666]

Oh, and my personal view is that bin/fba_tertiary_too and py/fiberassign/fba_tertiary_io.py belong in fiberassign not desitarget. I see the tertiary programs as "programs to which we want to assign fibers as rapidly as we can for special reasons." I think having desitarget process these programs and then passing them on to fiberassign for further processing slows down the overall procedure.

If a proposer of tertiary targets wants to "try out" and refine their selection before we process official tiles, I think that's easier if the whole procedure is handled by fiberassign, rather than iterating backwards-and-forwards between fiberassign and desitarget.

I appreciate that much of the past slowness for rapid-turnaround of special programs is ameliorated by using the ToO mechanism, though. So, I'm happy to revisit this and port parts of the code to desitarget if Stephen wants to make an executive decision. But, not on the timescale of the next few days (when we want to be designing Rongpu's program).

[araichoor]

thanks a lot @geordie666 for the careful review — and very relevant comments!
I ll address those after dinner here (not before a couple of hours).

but already one clarification, about the --desi_mask_name and --scnd_mask_name arguments in bin/fba_tertiary_too:
I had in mind that this code would be run by me (or someone from the surveyops team), not by the provider.
the provider just provides the tertiary-targets-PROGNUM.fits and tertiary-priorities-PROGNUM.ecsv files.
I added those arguments for possible flexibility in the future.

I agree that we could hard-code for now those in py/fiberassign/fba_tertiary_io.py.
and see later in future how to proceed if we d want to enable other mask names.

[geordie666]

I understand that about one of us running the pipeline — but there's always the possibility that we train someone else to use the software, or you and I are both on vacation, etc. Better to be safe, I think, and not allow any "unfortunate" uses of the code if we don't intend to be overly flexible.

[araichoor]

again, thanks a log @geordie666 for the review.
I think I ve addressed the more pressing comments; few remain open to discussion, I could implement those tomorrow morning, if you d like to (those are simple things to code).

besides, I ve also pushed few commits correcting small things:

• CHECKER -> bugfix, I was forgetting to actually copy the column;
• args.fadir -> bugfix, it was a required argument; now it s an optional argument;
• raise IOError instead of warning if a previous_tileids is not found.

lastly: cori has been more cooperative tonight, and I ve double-check that I can reproduce all main tiles.

[araichoor]

oh, and if informative, here how the content of ToO-PROGNUM-TILEID.ecsv file looks like:

the column format:

# ---
# datatype:
# - {name: RA, unit: deg, datatype: float64}
# - {name: DEC, unit: deg, datatype: float64}
# - {name: PMRA, unit: mas / yr, datatype: float64}
# - {name: PMDEC, unit: mas / yr, datatype: float64}
# - {name: REF_EPOCH, unit: yr, datatype: float64}
# - {name: FLUX_G, unit: nmgy, datatype: float32}
# - {name: FLUX_R, unit: nmgy, datatype: float32}
# - {name: FLUX_Z, unit: nmgy, datatype: float32}
# - {name: PARALLAX, unit: mas, datatype: float32}
# - {name: GAIA_PHOT_G_MEAN_MAG, unit: mag, datatype: float32}
# - {name: GAIA_PHOT_BP_MEAN_MAG, unit: mag, datatype: float32}
# - {name: GAIA_PHOT_RP_MEAN_MAG, unit: mag, datatype: float32}
# - {name: GAIA_ASTROMETRIC_EXCESS_NOISE, datatype: float32}
# - {name: TARGETID, datatype: int64}
# - {name: DESI_TARGET, datatype: int64}
# - {name: SCND_TARGET, datatype: int64}
# - {name: SCND_ORDER, datatype: int64}
# - {name: PRIORITY_INIT, datatype: int64}
# - {name: SUBPRIORITY, datatype: float64}
# - {name: NUMOBS_INIT, datatype: int64}
# - {name: OBSCONDITIONS, datatype: int64}
# - {name: CHECKER, datatype: string}
# - {name: TOO_TYPE, datatype: string}
# - {name: TOO_PRIO, datatype: string}
# - {name: OCLAYER, datatype: string}
# - {name: MJD_BEGIN, unit: d, datatype: float64}
# - {name: MJD_END, unit: d, datatype: float64}
# - {name: TOOID, datatype: int64}
# - {name: TIMESTAMP, unit: s, datatype: string}
# - {name: NUMOBS, datatype: int64}
# - {name: NUMOBS_MORE, datatype: int64}
# - {name: PRIORITY, datatype: int64}

the header:

>>> d = Table.read("/global/cfs/cdirs/desi/users/raichoor/fiberassign-cosmoslowz/20220418/ToO-0001-999901.ecsv")
>>> for key in d.meta: print(key, d.meta[key])
... 
DEPNAM00 python
DEPNAM01 numpy
DEPNAM02 scipy
DEPNAM03 astropy
DEPNAM04 yaml
DEPNAM05 matplotlib
DEPNAM06 fitsio
DEPNAM07 healpy
DEPNAM08 desiutil
DEPNAM09 desitarget
DEPNAM10 desimodel
DEPNAM11 fiberassign
DEPVER00 3.8.3
DEPVER01 1.19.1
DEPVER02 1.5.0
DEPVER03 4.0.1.post1
DEPVER04 5.3.1
DEPVER05 3.2.1
DEPVER06 1.1.2
DEPVER07 1.14.0
DEPVER08 3.2.2
DEPVER09 2.2.1
DEPVER10 0.16.0
DEPVER11 5.4.0.dev3369
EXTNAME TOO
FAPRGRM cosmoslowz
FA_GIT 5.4.0-52-gf8f49fd
FA_VER 5.4.0.dev3369
GOALTIME 1200
OBSCONDS DARK
PRIOFN /global/cfs/cdirs/desi/users/raichoor/fiberassign-cosmoslowz/20220418/tertiary-priorities-0001.ecsv
SBPROF ELG
TARGFN /global/cfs/cdirs/desi/users/raichoor/fiberassign-cosmoslowz/20220418/tertiary-targets-0001.fits
TOOARGS --fadir /global/cfs/cdirs/desi/users/raichoor/fiberassign-cosmoslowz/20220418 --previous_tileids 999900 --prognum 1 --targdir /global/cfs/cdirs/desi/users/raichoor/fiberassign-cosmoslowz/20220418 --tiledec 2.069 --tileid 999901 --tilera 150.061 --utc_time_mjd_begin 2020-01-01T00:00:00+00:00 --utc_time_mjd_end 2120-01-01T00:00:00+00:00

and one the first row:

for key in d.dtype.names: print(key, d[key][0])
... 
RA 149.09587913026658
DEC 3.3796312994662676
PMRA 0.0
PMDEC 0.0
REF_EPOCH 2015.5
FLUX_G 0.0
FLUX_R 0.0
FLUX_Z 0.0
PARALLAX 0.0
GAIA_PHOT_G_MEAN_MAG 0.0
GAIA_PHOT_BP_MEAN_MAG 0.0
GAIA_PHOT_RP_MEAN_MAG 0.0
GAIA_ASTROMETRIC_EXCESS_NOISE 0.0
TARGETID 39089837394887932
DESI_TARGET 4611686018427387904
SCND_TARGET 4611686018427387904
SCND_ORDER 1276
PRIORITY_INIT 9500
SUBPRIORITY 0.1915194503788923
NUMOBS_INIT 1
OBSCONDITIONS 1
CHECKER RZ
TOO_TYPE TILE
TOO_PRIO HI
OCLAYER DARK
MJD_BEGIN 58849.0
MJD_END 95373.0
TOOID 0
TIMESTAMP 2022-04-19T06:56:12+00:00
NUMOBS 1
NUMOBS_MORE 0
PRIORITY 8000

[geordie666]

This looks good to me, pending any feedback from Eddie.

[schlafly]

More checking my understanding than anything---do you want to assert that sel.sum() == 1 around here?

[araichoor]

you correctly understood the logic!
I m actually doing that check exactly (and few others) in assert_tertiary_prio(), which verifies that the file is as expected:

fiberassign/py/fiberassign/fba_tertiary_io.py

Lines 331 to 345 in e2f7cfc

	sel = (prio["TERTIARY_TARGET"] == tertiary_target) & (
	prio["NUMOBS_DONE_MIN"] == 0
	)
	if sel.sum() == 0:
	msg = "NUMOBS_DONE_MIN=0 case not set for TERTIARY_TARGET={}".format(
	tertiary_target
	)
	log.error(msg)
	raise IOError(msg)
	if sel.sum() > 1:
	msg = "NUMOBS_DONE_MIN=0 case appearing {} times for TERTIARY_TARGET={}".format(
	sel.sum(), tertiary_target
	)
	log.error(msg)
	raise IOError(msg)

(I also do a similar check for the tertiary-targets-PROGNUM.fits file).

thanks for looking so carefully at the code.

[schlafly]

I'm happy to merge.

[geordie666]

I'm happy to merge, too.

[araichoor]

thanks both for all the feedback.
I m giving a last look at the code.
@schlafly, in case you re connected, a minor detail:
I notice that I ve kept EXTNAME="TOO" for the ToO-PROGNUM-TILEID.ecsv file:

fiberassign/py/fiberassign/fba_tertiary_io.py

Line 633 in e2f7cfc

write_with_units(toofn, too.as_array(), extname="TOO", header=hdr, ecsv=True)

shall we want to set EXTNAME="TERTIARY" instead? or just keep "TOO"?

[schlafly]

I think for this ToO file that is the processed result of the tertiary- input file, it's best to keep this looking as much like a normal ToO file as possible. i.e., EXTNAME=ToO makes sense to me.

I guess I'd also add that since this is an ecsv file and can have only one extension, I'm not sure anyone will know what the EXTNAME is.

[araichoor]

ok perfect, let s keep EXTNAME="TOO" (and as you say it probably doesn t matter much as it s an .ecsv file).
I ve just pushed one last commit with cleaning the imports, once the checks are completed, I ll merge.