DM-8750 cleanup jointcal compiler warnings #43
Conversation
First dynamic_cast will raise bad_cast if something goes wrong. There's no nullptr test to do on it.
parejkoj
force-pushed
the
tickets/DM-8750
branch
3 times, most recently
from
d3dc5e9
to
f4b37e9
Compare
| * computed as: | ||
| * @f[ | ||
| * <chi2> + nSigmaCut + rms(chi2) | ||
| * @f] |
There was a problem hiding this comment.
Does this display ok? It's technically correct, but not very TeXy.
There was a problem hiding this comment.
Yeah, it looks decent. I copied it from AstrometryFit.cc and wrapped it in the @f[ to get rid of the doxygen errors about the <.
| * @param[in] nSigmaCut Number of sigma to select on. | ||
| * @param mSOutliers list of MeasuredStar outliers to populate | ||
| * @param fSOutliers list of FittedStar outliers to populate | ||
| * @param[in] measOrRef Which outliers to remove. One of "Meas", "Ref" or "Meas Ref". |
There was a problem hiding this comment.
Missing [out] tags for mSOutliers and fSOutliers.
include/lsst/jointcal/Gtransfo.h
Outdated
| @@ -1,6 +1,5 @@ | |||
| // -*- LSST-C++ -*- | |||
|
|
|||
| /// \file gtransfo.h | |||
| /// \brief Geometrical transformations (of 2D points) | |||
There was a problem hiding this comment.
Don't leave this a Doxygen comment; it will attach to the wrong thing (likely namespace pexExcept or namespace lsst).
| * | ||
| * @return Total number of outliers that were removed. | ||
| */ | ||
| unsigned findOutliers(double nSigmaCut, MeasuredStarList &mSOutliers, FittedStarList &fSOutliers, | ||
| const std::string &measOrRef = "Meas Ref") const; | ||
|
|
||
| //! Just removes outliers from the fit. No Refit done. |
There was a problem hiding this comment.
The //! style should be replaced with /// (and yes, Doxygen may handle them differently, so this matters a bit more than most style choices). Is there already a ticket for this?
There was a problem hiding this comment.
There is now! DM-10867
src/ConstrainedPolyModel.cc
Outdated
| @@ -191,6 +191,8 @@ std::shared_ptr<TanSipPix2RaDec> ConstrainedPolyModel::produceSipWcs(const CcdIm | |||
|
|
|||
| GtransfoPoly pix2Tp; | |||
| const GtransfoPoly &t1 = dynamic_cast<const GtransfoPoly &>(mapping->getTransfo1()); | |||
| // TODO: This line produces a warning on clang (t1 is always valid, unless the dynamic_cast raises | |||
| // bad_cast), but I'll deal with it as part of DM-10524 (hopefully removing the necessity of the casts). | |||
There was a problem hiding this comment.
I've clarified the comment.
| @@ -131,7 +131,7 @@ void SparseHisto4d::binLimits(const double x[4], const int iDim, double &xMin, d | |||
| int code = code_value(x); | |||
| double xCenter[4]; | |||
| inverse_code(code, xCenter); | |||
| xMin, xCenter[iDim] - 0.5 / _scale[iDim]; | |||
| xMin = xCenter[iDim] - 0.5 / _scale[iDim]; | |||
There was a problem hiding this comment.
How the heck did this ever compile???
There was a problem hiding this comment.
I'm not entirely sure what was going on there.
| @@ -68,7 +67,7 @@ void PhotometryFit::LSDerivatives(const CcdImage &ccdImage, TripletList &triplet | |||
| // tweak the measurement errors | |||
| double sigma = measuredStar.eflux; | |||
| #ifdef FUTURE | |||
| TweakPhotomMeasurementErrors(inPos, measuredStar, _posError); | |||
| TweakPhotomMeasurementErrors(inPos, measuredStar, _fluxError); | |||
There was a problem hiding this comment.
I don't think I understand this change. Where did it come from?
There was a problem hiding this comment.
There's no _posError in PhotometryFit, but there probably will be _fluxError in the future (it is currently unused, thus removed). I'm only removing the preprocessor blocks when I'm sure I won't need to reference them, and cleaning them up as I would other code.
There was a problem hiding this comment.
Ah, ok. It seemed odd that you were adding a reference to _fluxError here when you were removing it everywhere else.
Searched for warnings with both
clang-800.0.38on macOS andgcc 5.4.0-6ubuntu1~16.04.4on Linux.