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
DM-28389: Create a gen3 task to collate tract-level parquet tables for QA analyses #335
Conversation
pipelines/makeQaTractTables.yaml
Outdated
makeQaTractTables: | ||
class: lsst.analysis.drp.makeQaTractTables.MakeQaTractTablesTask | ||
config: | ||
filterMap: {"g": "HSC-G", "r": "HSC-R", "i": "HSC-I", "z": "HSC-Z", "y": "HSC-Y", "N921": "NB0921"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems wrong to me that we have to duplicate the mapping from band to physical filter here (and it doesn't have the second physical filters) since this information is already defined explicitly in obs_subaru in hscFilters.py and also directly discoverable from the butler itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that’s great! I would be very happy to be getting this from the butler and get rid of this file altogether 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use butler.registry.queryDimensionRecords
. From the command line you get something like:
$ butler query-dimension-records tmp physical_filter
instrument name band
---------- ----------------------------- -----------------------------
HSC ENG-R1 r1
HSC HSC-G g
HSC HSC-I i
HSC HSC-I2 i
HSC HSC-R r
HSC HSC-R2 r
HSC HSC-Y y
HSC HSC-Z z
HSC IB0945 I945
HSC NB0387 N387
HSC NB0400 N400
HSC NB0468 N468
HSC NB0515 N515
HSC NB0527 N527
HSC NB0656 N656
HSC NB0718 N718
HSC NB0816 N816
HSC NB0921 N921
HSC NB0926 N926
HSC NB0973 N973
HSC NB1010 N1010
HSC NONE UNRECOGNISED
HSC PH PH
HSC SH SH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect...thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can restrict the query to just the filter you are interested in for the instrument you use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly... I think the confusion was the thought that I was trying to switch to physical
rather than sticking with band
. This is not the case at all (I’m totally on board with just band
for any and all things coadd!). It’s just that I currently need physical
to get at the data in the _ojb
tables.
I have also been speculating that this’ll just naturally get fixed along with the Gen3-ification of the code making the _obj
tables, hence me very much referring to this as a temporary fix. I’m perfectly happy to hold off merging this until the fix is in so that this never shows up as part of the history on master (it shouldn’t be too painful for me to just run off the two branches manually for the intervening RC2 runs).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Nate. Yeah, I fully landed on NOT using ButlerQC.regitry
, but it did seem worthwhile pointing out that it could be done (and hence a trap others could fall into).
Ah, yes, I knew there had to be a way to reuse that config...thanks for the tip!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still seems fundamentally wrong to me that a pipeline file should define a filter-to-band mapping when we already have that mapping defined in two other places (such as in obs_subaru itself in the filter definitions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on what it is. I see nothing wrong with having a config file that can be imported and used anywhere you want "here is what is defined in obs_subaru" But also these are configurable fields, so if someone wants to configure them in a non-standard way for whatever their reason (emulate a wider band from a different camera etc) then they should be able to specify the config parameter to their liking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still seems fundamentally wrong to me that a pipeline file should define a filter-to-band mapping when we already have that mapping defined in two other places (such as in obs_subaru itself in the filter definitions).
It's not a physical_filter
-to-band
mapping; it's a band
-to-pseudo/historical-physical_filter
mapping, and we don't have one of those in obs_subaru because it's an ill-defined thing. Unfortunately it's exactly the ill-defined thing that's still in use in Gen2.
c907160
to
5f66d39
Compare
5f66d39
to
51871e9
Compare
The configs here add a filter mapping between the "bands" and the HSC "physical filters".
51871e9
to
ad12a6a
Compare
No description provided.