DM-12243: make inputCount more robust #99
Conversation
PaulPrice
force-pushed
the
tickets/DM-12243
branch
from
e1f2576
to
150813d
Compare
| @@ -50,7 +50,7 @@ def setDefaults(self): | |||
| self.measurement.copyColumns["id"] = "id" | |||
| self.measurement.copyColumns["parent"] = "parent" | |||
| self.references.removePatchOverlaps = False # see validate() for why | |||
| self.measurement.plugins.names |= ['base_InputCount'] | |||
| self.measurement.plugins.names |= ['base_InputCount', 'base_Variance'] | |||
There was a problem hiding this comment.
why do these get promoted to always be run, as opposed to camera configs or run specific configs? Is this something we thing everyone will want all the time?
python/lsst/meas/base/wrappers.py
Outdated
| def getTransformClass(self): | ||
| return self._generic.getTransformClass() | ||
|
|
||
| return ForcedFromGenericPlugin |
There was a problem hiding this comment.
I think I would prefer to see these as class methods on the generic plugin class called something like make<Single/forced>Plugin, so that something that is wrapped can just be singleFooPlugin = genericFooPluging.makeSingle("name"), each of these could then even deffer to a third more common code base to reduce duplication further if desired
There was a problem hiding this comment.
Sounds like a good idea. I'll give it a go.
python/lsst/meas/base/wrappers.py
Outdated
|
|
||
| def __init__(self, config, name, schema, metadata, logName=None): | ||
| SingleFramePlugin.__init__(self, config, name, schema, metadata, logName=logName) | ||
| self._generic = GenericPluginClass(config, name, schema, metadata) |
There was a problem hiding this comment.
I am nearly positive that if you are creating these classes inside a function the GenericPluginClass (or cls; see below) variable will be in a closure, and there is no need to explicitly store another pointer to it inside the self object.
There was a problem hiding this comment.
I'm storing an instance, not the class itself. I don't think we want to instantiate it every time we want to use it.
Or have I misunderstood your point?
| @@ -50,6 +50,7 @@ def setDefaults(self): | |||
| self.measurement.copyColumns["id"] = "id" | |||
| self.measurement.copyColumns["parent"] = "parent" | |||
| self.references.removePatchOverlaps = False # see validate() for why | |||
| self.measurement.plugins.names |= ['base_InputCount', 'base_Variance'] | |||
There was a problem hiding this comment.
why do these get promoted to always be run, as opposed to camera configs or run specific configs? Is this something we thing everyone will want all the time?
There was a problem hiding this comment.
Certainly the InputCount is something everyone will want to use (e.g., it can be used as a poor-man's detection limit). HSC experience is that the Variance is helpful too.
Having it only in unforced measurement isn't as helpful because it can be skipped if the centroid is flagged, leaving us without a count measurement.
Even if the centroid has been declared to be bad (through the flag), still attempt to measure the number of inputs at the position, as this might provide something useful.
PaulPrice
force-pushed
the
tickets/DM-12243
branch
from
150813d
to
8d80d44
Compare
|
Moved the factory functions into the |
PaulPrice
force-pushed
the
tickets/DM-12243
branch
2 times, most recently
from
731e41b
to
394599c
Compare
Some measurement plugins are really generic in the sense that it doesn't matter whether they are run as a single-frame or forced plugin, but the plumbing is slightly different (most notably, the signature of the 'measure' method). To work around this, added a GenericPlugin class and methods that will use that to produce the appropriate SingleFramePlugin and ForcedPlugin classes so common code can be used for both.
These previously only worked in single-frame measurement, but they would be useful in forced measurement too. Using the new GenericPlugin, we can use the same code to achieve both. This necessitated some small changes in the error handling.
PaulPrice
force-pushed
the
tickets/DM-12243
branch
from
394599c
to
c2f11c2
Compare
And run it in forced coadd measurement.