DM-25768: Remove computeFluxScale function #162
Conversation
taranu
force-pushed
the
tickets/DM-25768
branch
2 times, most recently
from
b1d7a44
to
51e65e7
Compare
src/SdssShape.cc
Outdated
| double IxxIyy = result.getQuadrupole().getIxx() * result.getQuadrupole().getIyy(); | ||
| double Ixy_sq = result.getQuadrupole().getIxy(); | ||
| Ixy_sq *= Ixy_sq; | ||
| if (IxxIyy < (1.0 + 1.0e-6) * Ixy_sq) |
There was a problem hiding this comment.
Could you make this:
epsilon = 1.0e-6
if (IxxIyy < (1.0 + epsilon) * Ixy_sq)
I like to see magic numbers defined on their own line, both to call attention to them and to make it obvious when they change.
src/SdssShape.cc
Outdated
| @@ -871,8 +857,8 @@ FluxResult SdssShapeAlgorithm::computeFixedMomentsFlux(ImageT const &image, | |||
| .str()); | |||
| } | |||
| double var = ImageAdaptor<ImageT>().getVariance(image, ix, iy); | |||
| double i0Err = std::sqrt(var / wArea); | |||
| result.instFluxErr = i0Err * 2 * wArea; | |||
| // i0Err = std::sqrt(var / wArea); instFluxErr = 2 * wArea * i0Err | |||
There was a problem hiding this comment.
Remove the commented-out code, but add a real comment to show where your final formula came from.
src/SdssShape.cc
Outdated
| double IxxIyy = result.getQuadrupole().getIxx() * result.getQuadrupole().getIyy(); | ||
| double Ixy_sq = result.getQuadrupole().getIxy(); | ||
| Ixy_sq *= Ixy_sq; | ||
| if (IxxIyy < (1.0 + 1.0e-6) * Ixy_sq) | ||
| // We are checking that Ixx*Iyy > (1 + epsilon)*Ixy*Ixy where epsilon is suitably small. The | ||
| // value of epsilon used here is a magic number. DM-5801 is supposed to figure out if we are | ||
| // to keep this value. |
There was a problem hiding this comment.
If you can think of a better error message than the one below, please do so. But, it is outside the scope of this ticket, so don't feel you have to. If I got that error while processing data, I'd have to search the code for the message in order to figure out what went wrong.
There was a problem hiding this comment.
I made it slightly more useful. I haven't ever seen it actually pop up; it should be impossible. Also noted that DM-5801 was marked won't fix long ago.
taranu
force-pushed
the
tickets/DM-25768
branch
from
51e65e7
to
ae37dd9
Compare
No description provided.