Skip to content

Commit

Permalink
Post-review cleanups
Browse files Browse the repository at this point in the history 
Add Chi2.cc pulled from Chi2.h in review

Fix many "const T &" -> "T const &"

Make minimize() return an enum type

Explicit delete or default copy/move for many classes

Have Fitter constructors set _nPar* variables to 0, instead of calling
overridden assignIndices(), which makes further sub-classes hard to create.

cleanup doxygen
  • Loading branch information
@parejkoj
parejkoj committed Aug 1, 2017
1 parent 69ad124 commit 447a761
Show file tree
Hide file tree
Showing 38 changed files with 632 additions and 507 deletions.
16 changes: 12 additions & 4 deletions include/lsst/jointcal/Associations.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,15 @@ class Associations {
* Source selection is performed in python, so Associations' constructor
* only initializes a couple of variables.
*/
Associations() : _commonTangentPoint(Point(0, 0)) {}
Associations()
: _commonTangentPoint(Point(std::numeric_limits<double>::quiet_NaN(),
std::numeric_limits<double>::quiet_NaN())) {}

/// No moves or copies: jointcal only ever needs one Associations object.
Associations(Associations const &) = delete;
Associations(Associations &&) = delete;
Associations &operator=(Associations const &) = delete;
Associations &operator=(Associations &&) = delete;

/**
* @brief Sets a shared tangent point for all ccdImages.
Expand Down Expand Up @@ -90,8 +98,8 @@ class Associations {
*/
void collectRefStars(lsst::afw::table::SortedCatalogT<lsst::afw::table::SimpleRecord> &refCat,
afw::geom::Angle matchCut, std::string const &fluxField,
std::map<std::string, std::vector<double> > const &refFluxMap,
std::map<std::string, std::vector<double> > const &refFluxErrMap);
std::map<std::string, std::vector<double>> const &refFluxMap,
std::map<std::string, std::vector<double>> const &refFluxErrMap);

//! Sends back the fitted stars coordinates on the sky FittedStarsList::inTangentPlaneCoordinates keeps
//! track of that.
Expand All @@ -111,7 +119,7 @@ class Associations {
*/
void selectFittedStars(int minMeasurements);

const CcdImageList &getCcdImageList() const { return ccdImageList; }
CcdImageList const &getCcdImageList() const { return ccdImageList; }

//! Number of different bands in the input image list. Not implemented so far
unsigned getNFilters() const { return _filterMap.size(); }
Expand Down
80 changes: 39 additions & 41 deletions include/lsst/jointcal/AstrometryFit.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,16 @@ namespace jointcal {
* have any Kind 2 terms.
*/
class AstrometryFit : public FitterBase {
private:
bool _fittingDistortions, _fittingPos, _fittingRefrac, _fittingPM;
AstrometryModel &_astrometryModel;
double _referenceColor, _sigCol; // average and r.m.s color
unsigned _nRefrac;
double _refractionCoefficient; // fit parameter
unsigned int _refracPosInMatrix; // where it stands
double _JDRef; // average Julian date

// counts in parameter subsets.
unsigned int _nParDistortions;
unsigned int _nParPositions;
double _posError; // constant term on error on position (in pixel unit)

public:
//! this is the only constructor
AstrometryFit(Associations &associations, AstrometryModel &astrometryModel, double posError);
AstrometryFit(std::shared_ptr<Associations> associations,
std::shared_ptr<AstrometryModel> astrometryModel, double posError);

/// No copy or move: there is only ever one fitter of a given type.
AstrometryFit(AstrometryFit const &) = delete;
AstrometryFit(AstrometryFit &&) = delete;
AstrometryFit &operator=(AstrometryFit const &) = delete;
AstrometryFit &operator=(AstrometryFit &&) = delete;

/**
* Set parameters to fit and assign indices in the big matrix.
Expand All @@ -89,7 +82,7 @@ class AstrometryFit : public FitterBase {
* subsets of distortion parameter on or off, if the
* AstrometryModel implements such a thing.
*/
void assignIndices(const std::string &whatToFit);
void assignIndices(std::string const &whatToFit) override;

/**
* The transformations used to propagate errors are freezed to the current
Expand All @@ -98,52 +91,57 @@ class AstrometryFit : public FitterBase {
* affected when updating the mappings. This allows to have an exactly linear
* fit, which can be useful.
*/
void freezeErrorScales() { _astrometryModel.freezeErrorScales(); }
void freezeErrorScales() { _astrometryModel->freezeErrorScales(); }

/**
* Offset the parameters by the requested quantities. The used parameter
* layout is the one from the last call to assignIndices or minimize().
* There is no easy way to check that the current setting of whatToFit and
* the provided Delta vector are compatible. We can only test the size.
*
* @param[in] delta vector of offsets to apply
*/
void offsetParams(const Eigen::VectorXd &delta);
void offsetParams(Eigen::VectorXd const &delta) override;

void saveResultTuples(const std::string &tupleName) const;
void saveResultTuples(std::string const &tupleName) const override;

//! Produces a tuple containing residuals of measurement terms.
void makeMeasResTuple(const std::string &tupleName) const;
void makeMeasResTuple(std::string const &tupleName) const;

//! Produces a tuple containing residuals of reference terms.
void makeRefResTuple(const std::string &tupleName) const;
void makeRefResTuple(std::string const &tupleName) const;

/**
* DEBUGGING routine
*/
void checkStuff();

private:
// void leastSquareDerivatives(TripletList &tripletList, Eigen::VectorXd &grad) const;
bool _fittingDistortions, _fittingPos, _fittingRefrac, _fittingPM;
std::shared_ptr<AstrometryModel> _astrometryModel;
double _referenceColor, _sigCol; // average and r.m.s color
double _refractionCoefficient; // fit parameter
unsigned int _refracPosInMatrix; // where it stands
double _JDRef; // average Julian date

// counts in parameter subsets.
unsigned int _nParDistortions;
unsigned int _nParPositions;
unsigned int _nParRefrac;

void leastSquareDerivativesMeasurement(const CcdImage &ccdImage, TripletList &tripletList,
double _posError; // constant term on error on position (in pixel unit)

void leastSquareDerivativesMeasurement(CcdImage const &ccdImage, TripletList &tripletList,
Eigen::VectorXd &grad,
const MeasuredStarList *msList = nullptr) const;
MeasuredStarList const *msList = nullptr) const override;

void leastSquareDerivativesReference(const FittedStarList &fittedStarList, TripletList &tripletList,
Eigen::VectorXd &grad) const;
void leastSquareDerivativesReference(FittedStarList const &fittedStarList, TripletList &tripletList,
Eigen::VectorXd &grad) const override;

Point transformFittedStar(const FittedStar &fittedStar, const Gtransfo *sky2TP,
const Point &refractionVector, double refractionCoeff, double mjd) const;
void accumulateStatImageList(CcdImageList const &ccdImageList, Chi2Accumulator &accum) const override;

void accumulateStatImageList(CcdImageList const &ccdImageList, Chi2Accumulator &accum) const;
void accumulateStatRefStars(Chi2Accumulator &accum) const override;

void accumulateStatImage(CcdImage const &ccdImage, Chi2Accumulator &accum) const;
void getIndicesOfMeasuredStar(MeasuredStar const &measuredStar,
std::vector<unsigned> &indices) const override;

void accumulateStatRefStars(Chi2Accumulator &accum) const;
Point transformFittedStar(FittedStar const &fittedStar, Gtransfo const *sky2TP,
Point const &refractionVector, double refractionCoeff, double mjd) const;

//! only for outlier removal
void setMeasuredStarIndices(const MeasuredStar &ms, std::vector<unsigned> &indices) const;
/// Compute the chi2 (per star or total, depending on which Chi2Accumulator is used) from one CcdImage.
void accumulateStatImage(CcdImage const &ccdImage, Chi2Accumulator &accum) const;
};
} // namespace jointcal
} // namespace lsst
Expand Down
10 changes: 5 additions & 5 deletions include/lsst/jointcal/AstrometryModel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,24 @@ the top of simplepolymodel.h */
class AstrometryModel {
public:
//! Mapping associated to a given CcdImage
virtual const Mapping *getMapping(const CcdImage &) const = 0;
virtual const Mapping *getMapping(CcdImage const &) const = 0;

//! Assign indices to parameters involved in mappings, starting at firstIndex. Returns the highest
//! assigned index.
virtual unsigned assignIndices(unsigned firstIndex, const std::string &whatToFit) = 0;
virtual unsigned assignIndices(unsigned firstIndex, std::string const &whatToFit) = 0;

//! Offset the parameters by the provided amounts.
/*! The shifts are applied according to the indices given in
AssignIndices. */
virtual void offsetParams(const Eigen::VectorXd &delta) = 0;
virtual void offsetParams(Eigen::VectorXd const &delta) = 0;

//! The transformation used to project the positions of FittedStars.
/*! This defines the coordinate system into which the Mapping of
this Ccdimage maps the pixel coordinates. */
virtual const Gtransfo *getSky2TP(const CcdImage &ccdImage) const = 0;
virtual const Gtransfo *getSky2TP(CcdImage const &ccdImage) const = 0;

//! Cook up a SIP WCS.
virtual std::shared_ptr<TanSipPix2RaDec> produceSipWcs(const CcdImage &ccdImage) const = 0;
virtual std::shared_ptr<TanSipPix2RaDec> produceSipWcs(CcdImage const &ccdImage) const = 0;

//!
virtual void freezeErrorScales() = 0;
Expand Down
11 changes: 5 additions & 6 deletions include/lsst/jointcal/BaseStar.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class BaseStar : public FatPoint {
//! constructor
BaseStar(double xx, double yy, double flux, double fluxErr)
: FatPoint(xx, yy), _flux(flux), _fluxErr(fluxErr){};
BaseStar(const Point &point, double flux, double fluxErr)
BaseStar(Point const &point, double flux, double fluxErr)
: FatPoint(point), _flux(flux), _fluxErr(fluxErr){};

//! access stuff.
Expand All @@ -43,7 +43,7 @@ class BaseStar : public FatPoint {
double getY() const { return y; }

//! allows std::cout << aBaseStar;
friend std::ostream &operator<<(std::ostream &stream, const BaseStar &s) {
friend std::ostream &operator<<(std::ostream &stream, BaseStar const &s) {
s.dump(stream);
return stream;
}
Expand All @@ -58,7 +58,7 @@ class BaseStar : public FatPoint {
stream << "x: " << x << " y: " << y << " flux: " << _flux << " fluxErr: " << _fluxErr;
}

BaseStar &operator=(const Point &point) {
BaseStar &operator=(Point const &point) {
this->x = point.x;
this->y = point.y;
return (*this);
Expand All @@ -73,14 +73,13 @@ class BaseStar : public FatPoint {
void setFlux(double flux) { _flux = flux; }

double getFluxErr() const { return _fluxErr; }
// double &getFluxErr() { return _fluxErr; }
void setFluxErr(double fluxErr) { _fluxErr = fluxErr; }
};

//! enables to sort easily a starList (of anything that derives from BaseStar)
bool decreasingFlux(const BaseStar *star1, const BaseStar *star2);
bool decreasingFlux(BaseStar const *star1, BaseStar const *star2);

int decodeFormat(const char *formatLine, const char *starName);
int decodeFormat(char const *formatLine, char const *starName);

typedef StarList<BaseStar> BaseStarList;

Expand Down
46 changes: 24 additions & 22 deletions include/lsst/jointcal/CcdImage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
namespace lsst {
namespace jointcal {

typedef std::list<std::shared_ptr<CcdImage> > CcdImageList;
typedef std::list<std::shared_ptr<CcdImage>> CcdImageList;

typedef int VisitIdType;
typedef int CcdIdType;
Expand Down Expand Up @@ -64,15 +64,20 @@ class CcdImage {

Point _commonTangentPoint;

void LoadCatalog(const lsst::afw::table::SortedCatalogT<lsst::afw::table::SourceRecord> &Cat,
const std::string &fluxField);
void LoadCatalog(lsst::afw::table::SortedCatalogT<lsst::afw::table::SourceRecord> const &Cat,
std::string const &fluxField);

public:
CcdImage(lsst::afw::table::SortedCatalogT<lsst::afw::table::SourceRecord> &record,
const PTR(lsst::afw::image::TanWcs) wcs, const PTR(lsst::afw::image::VisitInfo) visitInfo,
const lsst::afw::geom::Box2I &bbox, const std::string &filter,
const std::shared_ptr<afw::image::PhotoCalib> photoCalib, const int &visit, const int &ccd,
const std::string &fluxField);
CcdImage(afw::table::SourceCatalog &record, std::shared_ptr<lsst::afw::image::TanWcs> wcs,
std::shared_ptr<lsst::afw::image::VisitInfo> visitInfo, afw::geom::Box2I const &bbox,
std::string const &filter, std::shared_ptr<afw::image::PhotoCalib> photoCalib, int visit,
int ccd, std::string const &fluxField);

/// No move or copy: each CCD image is unique to that ccd+visit, and Associations holds all CcdImages.
CcdImage(CcdImage const &) = delete;
CcdImage(CcdImage &&) = delete;
CcdImage &operator=(CcdImage const &) = delete;
CcdImage &operator=(CcdImage &&) = delete;

//! Return the _name that identifies this ccdImage.
std::string getName() const { return _name; }
Expand All @@ -82,15 +87,15 @@ class CcdImage {
*
* @return The whole catalog.
*/
const MeasuredStarList &getWholeCatalog() const { return _wholeCatalog; }
MeasuredStarList const &getWholeCatalog() const { return _wholeCatalog; }

//@{
/**
* @brief Gets the catalog to be used for fitting, which may have been cleaned-up.
*
* @return The catalog for fitting.
*/
const MeasuredStarList &getCatalogForFit() const { return _catalogForFit; }
MeasuredStarList const &getCatalogForFit() const { return _catalogForFit; }
MeasuredStarList &getCatalogForFit() { return _catalogForFit; }
//@}

Expand All @@ -99,7 +104,7 @@ class CcdImage {
*
* @param[in] commonTangentPoint The common tangent point of all ccdImages (decimal degrees).
*/
void setCommonTangentPoint(const Point &commonTangentPoint);
void setCommonTangentPoint(Point const &commonTangentPoint);

/**
* @brief Gets the common tangent point, shared between all ccdImages.
Expand All @@ -109,19 +114,19 @@ class CcdImage {
Point const &getCommonTangentPoint() const { return _commonTangentPoint; }

//!
const Gtransfo *getPix2CommonTangentPlane() const { return _pix2CommonTangentPlane.get(); }
Gtransfo const *getPix2CommonTangentPlane() const { return _pix2CommonTangentPlane.get(); }

//!
const Gtransfo *getCommonTangentPlane2TP() const { return _CTP2TP.get(); }
Gtransfo const *getCommonTangentPlane2TP() const { return _CTP2TP.get(); }

//!
const Gtransfo *getTP2CommonTangentPlane() const { return _TP2CTP.get(); }
Gtransfo const *getTP2CommonTangentPlane() const { return _TP2CTP.get(); }

//!
const Gtransfo *getPix2TangentPlane() const { return _pix2TP.get(); }
Gtransfo const *getPix2TangentPlane() const { return _pix2TP.get(); }

//!
const Gtransfo *getSky2TP() const { return _sky2TP.get(); }
Gtransfo const *getSky2TP() const { return _sky2TP.get(); }

//! returns ccd ID
int getCcdId() const { return _ccdId; }
Expand Down Expand Up @@ -162,16 +167,13 @@ class CcdImage {
std::string getFilter() const { return _filter; }

//! the wcs read in the header. NOT updated when fitting.
const Gtransfo *readWCS() const { return _readWcs.get(); }
Gtransfo const *readWCS() const { return _readWcs.get(); }

//! the inverse of the one above.
const Gtransfo *getInverseReadWCS() const { return _inverseReadWcs.get(); }
Gtransfo const *getInverseReadWCS() const { return _inverseReadWcs.get(); }

//! Frame in pixels
const Frame &getImageFrame() const { return _imageFrame; }

private:
CcdImage(const CcdImage &); // forbid copies
Frame const &getImageFrame() const { return _imageFrame; }
};
} // namespace jointcal
} // namespace lsst
Expand Down

0 comments on commit 447a761

Please sign in to comment.