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

meas_algorithms/DM-8147: Add backwards compatibility to CoaddPsf input. #52

Merged
merged 1 commit into from Nov 3, 2016

Conversation

TallJimbo
Copy link
Member

This adds support for reading very old Exposures with CoaddPsfs written before 7/10/2013. It will not in general preserve the PSF's "average position" exactly, which is used when no position is provided to the PSF, because the information needed to reconstruct that has already been lost.

CoaddPsfPersistenceHelper const & keys1 = CoaddPsfPersistenceHelper::get();
LSST_ARCHIVE_ASSERT(catalogs.size() == 2u);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you keep this now that you have the check above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Will do.

Copy link
Contributor

@r-owen r-owen left a comment

Choose a reason for hiding this comment

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

Looks fine, though I would like one explanatory code comment. (I also wish catalogs had an explicit version # saved with them, but that's another ticket).

@@ -354,8 +354,11 @@ class CoaddPsf::Factory : public tbl::io::PersistableFactory {

virtual PTR(tbl::io::Persistable)
read(InputArchive const & archive, CatalogVector const & catalogs) const {
CoaddPsfPersistenceHelper const & keys1 = CoaddPsfPersistenceHelper::get();
if (catalogs.size() == 1u) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please explain this test (why does catalogs.size() tell us the catalog version) in a code comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

This adds support for reading very old Exposures with CoaddPsfs
written before 7/10/2013.  It will not in general preserve the
PSF's "average position" exactly, which is used when no position is
provided to the PSF, because the information needed to reconstruct
that has already been lost.
@TallJimbo TallJimbo merged commit ce948d9 into master Nov 3, 2016
@ktlim ktlim deleted the tickets/DM-8147 branch August 25, 2018 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants