DM-18189: Investigate mapping of ip_diffim dipole fields to DPDD #45
Conversation
| @@ -230,16 +264,30 @@ def run(self, inputCatalog, exposure): | |||
|
|
|||
| return outputCatalog | |||
|
|
|||
| def computeSnr(self, inputRecord, outputRecord): | |||
There was a problem hiding this comment.
Since the DPDD wants this to be detection SNR specifically, using the measurements in existing catalogs this way is misleading. I'd suggest simply removing this (and related SNR pieces) for now.
There was a problem hiding this comment.
What about using PeakFlux? Would that be an acceptable middle ground while not perfectly being the detection snr?
There was a problem hiding this comment.
As we discussed in the ap_pipe, I'll remove this to avoid any confusion between the what the value here means the one requested by the Ppdb.
| "the dipole.", | ||
| default="ip_diffim_DipoleFit_separation" | ||
| ) | ||
| snrFlux = pexConfig.Field( |
There was a problem hiding this comment.
(as below, let's strike this for now)
| @@ -217,7 +248,10 @@ def run(self, inputCatalog, exposure): | |||
| for inputRecord in inputCatalog: | |||
| outputRecord = outputCatalog.addNew() | |||
| outputRecord.assign(inputRecord, self.mapper) | |||
| self.computeSnr(inputRecord, outputRecord) | |||
| np.sqrt(neg_meas.error ** 2 + pos_meas.error ** 2)) | ||
|
|
||
| def computeDipoleSep(self, inputRecord, outputRecord, wcs): | ||
| """Calibrate and compute dipole mean flux and diff flux. |
There was a problem hiding this comment.
Shouldn't this docstring be "compute dipole separation"?
There was a problem hiding this comment.
Thanks, for catching this. I copy pasted the previous method's docs and forgot to change the description.
morriscb
force-pushed
the
tickets/DM-18189
branch
from
da156dd
to
375c819
Compare
Match ipdiffim catalog typing. Add new dipole columns to tests. Add columns and tests for dipFlux Add isDipole columne Add length to unittest. Debug unittest and typing. Add SNR calculation and test. Remove SNR calculations and fix up dipSep doc string. Remove remaining snr calculation uses.
morriscb
force-pushed
the
tickets/DM-18189
branch
from
f63f028
to
e5bc95e
Compare
No description provided.