DM-32252: Add variance_median and improve test coverage. #197
Conversation
|
|
||
| We compare the instFlux inside and outside source Footprints on an | ||
| extremely high S/N image. | ||
| """ | ||
| # We choose a random seed which causes the test to pass. |
There was a problem hiding this comment.
Slightly scary comment - is it super sensitive to this? randomSeed=0 doesn't seem like you had to tune it too much, or just got very lucky. Can you make this sound a little less scary?
There was a problem hiding this comment.
This is just cut and paste from the original test. I actually don't know the story...
tests/test_NoiseReplacer.py
Outdated
| # N.B. Next line checks that a random value is correct to a | ||
| # statistical 1-sigma prediction; some RNG seeds may cause it to | ||
| # fail (indeed, 67% should) | ||
| self.assertLess(record.get("test_NoiseReplacer_outside"), np.sqrt(sumVariance)) |
There was a problem hiding this comment.
Ah, OK, I see. Is there a method that also returns many values, that you can then do a slightly more thorough test on, like checking their mean and stddev?
tests/test_NoiseReplacer.py
Outdated
| def testVarianceNoiseReplacer(self): | ||
| """Test noise replacement in SFM with ''variance'' mode. | ||
| """ | ||
| # We choose a random seed which causes the test to pass. | ||
| task = self.makeSingleFrameMeasurementTask("test_NoiseReplacer") | ||
| task.config.noiseReplacer.noiseSource = 'variance' | ||
| exposure, catalog = self.dataset.realize(1.0, task.schema, randomSeed=0) | ||
| task.run(catalog, exposure) | ||
| sumVariance = exposure.getMaskedImage().getVariance().getArray().sum() | ||
| for record in catalog: | ||
| self.assertFloatsAlmostEqual(record.get("test_NoiseReplacer_inside"), | ||
| record.get("truth_instFlux"), rtol=1E-3) | ||
|
|
||
| # N.B. Next line checks that a random value is correct to a | ||
| # statistical 1-sigma prediction; some RNG seeds may cause it to | ||
| # fail (indeed, 67% should) | ||
| self.assertLess(record.get("test_NoiseReplacer_outside"), np.sqrt(sumVariance)) |
There was a problem hiding this comment.
Isn't this just an exact copy and paste of the test above, meaning you could do the whole thing with
for noiseSource in ['measure', 'variance']:
task.config.noiseReplacer.noiseSource = noiseSource```
tests/test_NoiseReplacer.py
Outdated
| # We choose a random seed which causes the test to pass. | ||
| task = self.makeSingleFrameMeasurementTask("test_NoiseReplacer") | ||
| # This selects the VariancePlaneNoiseReplacer. | ||
| task.config.noiseReplacer.noiseSource = 'variance_median' |
There was a problem hiding this comment.
This one also looks like it could go in the single test loop over a list comp one above too, right?
tests/test_NoiseReplacer.py
Outdated
| # N.B. Next line checks that a random value is correct to a | ||
| # statistical 1-sigma prediction; some RNG seeds may cause it to | ||
| # fail (indeed, 67% should) |
There was a problem hiding this comment.
OK, so this comment claims that 67% should fail statistically, but there's actually been no fine tuning there - all the random seeds are zero, and all apparently pass. What's up with that? (Yes, it could be luck, but still, worth wondering here).
erykoff
force-pushed
the
tickets/DM-32252
branch
from
1977b91
to
8de8f5d
Compare
No description provided.