Skip to content
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-41635: Add method to output model uncertainty #14

Merged
merged 1 commit into from Dec 13, 2023

Conversation

cmsaunders
Copy link
Collaborator

@cmsaunders cmsaunders commented Nov 7, 2023

The added function, CoordAlign::calculateAlphaInv() is based on what is done in the first 60 lines of the CoordAlign::fitOnce function, with the addition of calculating the inverse of the alpha matrix at the end.

Copy link

@taranu taranu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks fine. Can you add test coverage for the new functions?


llt = new LLT(alpha);
if (llt->info() == Eigen::NumericalIssue) choleskyFails = true;
#endif
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does there need to be an else here, or is one of USE_TMV and USE_EIGEN guaranteed to be true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the build requires that one of them be true.

@cmsaunders
Copy link
Collaborator Author

So, this is a vendored that doesn't have working testing (despite the existence of the /tests directory). We are basically using the tests in drp_tasks to keep gbdes tested.

@cmsaunders cmsaunders merged commit 8e52395 into lsst-dev Dec 13, 2023
@cmsaunders cmsaunders deleted the tickets/DM-41635 branch December 13, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants