-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DM-15011 Add ModelVisit/ModelChip options for photometry #96
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,14 +72,6 @@ class PhotometryMappingBase { | |
virtual void computeParameterDerivatives(MeasuredStar const &measuredStar, double instFlux, | ||
Eigen::Ref<Eigen::VectorXd> derivatives) const = 0; | ||
|
||
/** | ||
* Offset the transfo parameters by delta. | ||
* | ||
* @param[in] delta vector to offset transfo parameters. Same ordering as derivatives in | ||
* computeParameterDerivatives(); | ||
*/ | ||
virtual void offsetParams(Eigen::VectorXd const &delta) = 0; | ||
|
||
/// Make this mapping's parameters fixed (i.e. not varied during fitting). | ||
void setFixed(bool _fixed) { fixed = _fixed; } | ||
bool isFixed() { return fixed; } | ||
|
@@ -155,8 +147,13 @@ class PhotometryMapping : public PhotometryMappingBase { | |
} | ||
} | ||
|
||
/// @copydoc PhotometryMappingBase::offsetParams | ||
void offsetParams(Eigen::VectorXd const &delta) override { _transfo->offsetParams(delta); } | ||
/** | ||
* Offset the transfo parameters by delta. | ||
* | ||
* @param[in] delta vector to offset transfo parameters. Same ordering as derivatives in | ||
* computeParameterDerivatives(); | ||
*/ | ||
void offsetParams(Eigen::VectorXd const &delta) { _transfo->offsetParams(delta); } | ||
|
||
/// @copydoc PhotometryMappingBase::getParameters | ||
Eigen::VectorXd getParameters() override { return _transfo->getParameters(); } | ||
|
@@ -194,11 +191,13 @@ class ChipVisitPhotometryMapping : public PhotometryMappingBase { | |
ChipVisitPhotometryMapping(std::shared_ptr<PhotometryMapping> chipMapping, | ||
std::shared_ptr<PhotometryMapping> visitMapping) | ||
: PhotometryMappingBase(), | ||
_nParChips(0), | ||
_nParVisits(0), | ||
_chipMapping(std::move(chipMapping)), | ||
_visitMapping(std::move(visitMapping)) {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add explicit initialization of |
||
|
||
/// @copydoc PhotometryMappingBase::getNpar | ||
unsigned getNpar() const override { return _chipMapping->getNpar() + _visitMapping->getNpar(); } | ||
unsigned getNpar() const override { return _nParChips + _nParVisits; } | ||
|
||
/// @copydoc PhotometryMappingBase::transform | ||
double transform(MeasuredStar const &measuredStar, double instFlux) const override { | ||
|
@@ -224,12 +223,6 @@ class ChipVisitPhotometryMapping : public PhotometryMappingBase { | |
void computeParameterDerivatives(MeasuredStar const &measuredStar, double instFlux, | ||
Eigen::Ref<Eigen::VectorXd> derivatives) const override; | ||
|
||
/// @copydoc PhotometryMappingBase::offsetParams | ||
void offsetParams(Eigen::VectorXd const &delta) override { | ||
_chipMapping->offsetParams(delta.segment(0, _chipMapping->getNpar())); | ||
_visitMapping->offsetParams(delta.segment(_chipMapping->getNpar(), _visitMapping->getNpar())); | ||
} | ||
|
||
/// @copydoc PhotometryMappingBase::getParameters | ||
Eigen::VectorXd getParameters() override { | ||
Eigen::VectorXd joined(getNpar()); | ||
|
@@ -240,6 +233,17 @@ class ChipVisitPhotometryMapping : public PhotometryMappingBase { | |
/// @copydoc PhotometryMappingBase::getMappingIndices | ||
void getMappingIndices(std::vector<unsigned> &indices) const override; | ||
|
||
/** | ||
* Set whether to fit chips or visits. | ||
* | ||
* This must be called before anything that depends on knowing the number of parameters in the fit, | ||
* such as offsetParams(), getParameters(), or computeParameterDerivatives(). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sounds like something that should be handled by the constructor. (Though I think you gave me some argument for why you can't do that.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can't be done in the constructor, because |
||
* | ||
* @param fittingChips Fit the chip transform. | ||
* @param fittingVisits Fit the visit transform. | ||
*/ | ||
void setWhatToFit(bool const fittingChips, bool const fittingVisits); | ||
|
||
/// @copydoc PhotometryMappingBase::dump | ||
void dump(std::ostream &stream = std::cout) const override { | ||
stream << "index: " << index << " chipMapping: "; | ||
|
@@ -252,6 +256,9 @@ class ChipVisitPhotometryMapping : public PhotometryMappingBase { | |
std::shared_ptr<PhotometryMapping> getVisitMapping() const { return _visitMapping; } | ||
|
||
private: | ||
// These are either transfo.getNpar() or 0, depending on whether we are fitting that component or not. | ||
unsigned _nParChips, _nParVisits; | ||
|
||
// the actual transformation to be fit | ||
std::shared_ptr<PhotometryMapping> _chipMapping; | ||
std::shared_ptr<PhotometryMapping> _visitMapping; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is
_chipVisitMap
an optimization to avoid recomputing the composition? It might be good to explicitly spell out the self-consistency requirements (which I assume are enforced byConstrainedPhotometryModel
).