GeminiDRSoftware / DRAGONS

Data Reduction for Astronomy from Gemini Observatory North and South
Other
27 stars 16 forks source link

normalize_ucals sanity check fails due to tags assumption #13

Open marc-white opened 5 years ago

marc-white commented 5 years ago

In recipe_system.utils.reduce_utils.normalize_ucals, at line 380, a couple of sanity tests are performed. The first of these currently assumes that the tag set for a processed calibrator matches the calibrator name; e.g., it assumes that a 'processed_bias' will have tags ['PROCESSED', 'BIAS'] as part of its tag set.

This assumption breaks down for GHOST in two places: 'processed_slit' should have tags ['PROCESSED', 'SLIT*V] 'processed_slitflat' should have tags ['PROCESSED', 'SLITV', 'FLAT']

normalize_ucals (which is needed to use the Calibration Manager API) currently fails on at least 'processed_slitflat' because of this. I've temporarily solved the problem by simply commenting the test out, which isn't too dire (all the tests do is stop you making really silly mistakes when you're assigning calibrations programatically). However, it probably needs adjusting to actually read in the tag associations for each calibration type and instrument (and I can't recall where those live).

kiloRomeoAlpha commented 5 years ago

@marc-white Marc,

Yes, this will need a bit of a think. The initial implementation, as you can probably tell, is a bit simplistic. But it was really all we needed for extant Gemini cal types.

Initially, I had considered that a user should not have to tell reduce what kind of processed calibration is being passed -- that astrodata knows this, but this still requires hunting through a tagset looking for some one of a set of known enumerated calibration types, something like,

    if ad.tags.intersect(Set(['FLAT', 'BIAS', 'ARC', 'DARK'])):
        ...  then fiddle ...

_[Digression: I'm still looking to do this, which will remove the need for the user to pass a processed_xxxxx: signifier on the command line. That is not going to solve this GHOST tags problem, however.]_

I wonder if you cannot simply use calibration type, 'FLAT', for all your FLAT calibration types. For example, we have TWILIGHT FLATS, DOME FLATS, GCALFLATS, but when processed, all are simply called a processed_flat and then passed. Any primitives looking for such calibrations ought to know what their looking for. Perhaps this won't work for GHOST? as it seems like you might have a number of different FLAT types that need to be distinguished as such, but the tags will indicate what kind of FLAT has been passed.

If this is the case that GHOST needs multiple different FLAT types, searches on such cal types are degenerate in the archive, which has a fixed and enumerated set of Observation types (see archive Obs. Type: menu), only one of which is FLAT. Querying for FLAT returns all types of FLATS. It is possible to find, say, TWILIGHT flats by searching for Target Name: as Twilight. That will depend on what the descriptor object() returns. In fact, I think this would work if GHOST simply used processed_flat for all flats, providing that the target name (OBJECT in Gemini phu) is slitv.

The other action you might take on the GHOST side, is to simply define a tag to be slitflat for these calibration data. But, I'm not at all clear on whether that is viable or not. And, as you say, that does sort of break our "rule-ish" notion about what a tag should be.

I think we'll need to have some discussion about this with the rest of DRAGONS.

Thoughts?

kiloRomeoAlpha commented 5 years ago

Hi @marc-white,

After some discussion, and as a first cut perhaps to a more fully worked solution, I think we'll just remove the "sanity" check and let in whatever is passed as --user_cal. reduce will no longer try to ensure the user is passing the correct type, which has always seemed a tad paternalistic.

(I still think that you can and should pass these flat types as processed_flat and let your primitives then ask for the cal file they know they need. But, with the check on user_cal removed, or soon to be removed, you can ignore me and do what you prefer.)

This issue has raised associated concerns regarding GHOST data in the archive, and to make sure that GHOST observation/calibration types are accommodated by FitsStorage.