-
Notifications
You must be signed in to change notification settings - Fork 22
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-41345: handle non-finite positions when calculating covariance #718
Conversation
d88df1e
to
728f35e
Compare
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.
Looks good to me. Thanks for cleaning up the rest of the function--I obviously forgot to run anything to check formatting on DM-15180!
Eigen::Matrix2d localMatrix = measurementToLocalGnomonic->getJacobian(center); | ||
Eigen::Matrix2f d = localMatrix.cast < float > () * scale * (lsst::geom::PI / 180.0); |
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.
I don't know how those spaces got merged! Oops.
Eigen::Matrix2f calculateCoordCovariance(geom::SkyWcs const& wcs, lsst::geom::Point2D center, Eigen::Matrix2f err) { | ||
Eigen::Matrix2f calculateCoordCovariance(geom::SkyWcs const &wcs, lsst::geom::Point2D center, | ||
Eigen::Matrix2f err) { | ||
if (!std::isfinite(center.getX()) || !std::isfinite(center.getY())) { |
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.
I think this would be the end result anyway, but I agree it's better to just skip going forward in this situation.
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.
No, if the center is non-finite, the computed skyCenter
will be NaN and the call to geom::makeSkyWcs
will raise because it's not a valid WCS.
If either position is NaN, the `makeSkyWcs` call further down will fail because `skyCenter` is also NaN.
728f35e
to
2e4cf33
Compare
DM-41345: handle non-finite positions when calculating covariance
No description provided.