DM-43334: Add dmL1AstroErr/dmL2AstroErr metric #222
Conversation
There was a problem hiding this comment.
I agree with your choice of implementation. I confirmed that the code runs fine and produces reasonable results (though note that you'll need to change "RA" to "Dec" in the matchedVisitCore pipeline). Just a few small comments, and I think this is good to go!
| self.process.buildActions.perGroupStd.outUnit = "marcsec" | ||
| self.process.buildActions.perGroupStd.buildAction = PerGroupStatistic() | ||
| self.process.buildActions.perGroupStd.buildAction.func = "std" | ||
| self.process.buildActions.perGroupStd.buildAction.buildAction.vectorKey = "coord_dec" |
There was a problem hiding this comment.
I'm not sure I understand the use of "coord_dec" here -- is it just that it doesn't matter at this stage whether you give it RA or Dec because it gets overridden in .finalize()?
There was a problem hiding this comment.
Something has to be set, or the configuration validation fails. However, if I have this, I don't actually need the else part of the if self.coordinate == "RA":.. statement in finalize(), so I took that out.
| """Calculate the median position RMS of point sources.""" | ||
|
|
||
| fluxType = Field[str](doc="Flux type to calculate repeatability with", default="psfFlux") | ||
| level = Field[int](doc="Level 1 or 2 data product (1 or 2)", default=1) |
There was a problem hiding this comment.
May be worth documenting the fact that this config param doesn't actually set which level the task is run on, but just provides text for the metric name?
There was a problem hiding this comment.
Yes, good point! That should be more clear.
| atools.stellarAstrometricSelfRepeatabilityRA.coordinate: 'RA' | ||
| atools.stellarAstrometricSelfRepeatabilityDec: AstrometricRepeatability | ||
| atools.stellarAstrometricSelfRepeatabilityDec.level: 2 | ||
| atools.stellarAstrometricSelfRepeatabilityDec.coordinate: 'RA' |
cmsaunders
force-pushed
the
tickets/DM-43334
branch
2 times, most recently
from
954bab0
to
50819ea
Compare
cmsaunders
force-pushed
the
tickets/DM-43334
branch
from
50819ea
to
ea18701
Compare
No description provided.