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

obs_subaru/DM-7884: unification of coadd and diff datasets #40

Merged
merged 3 commits into from Oct 30, 2016

Conversation

TallJimbo
Copy link
Member

No description provided.

deepCoadd_depth, deepCoadd_bg, deepCoadd_bgRef, deepCoadd_forced + _schema
modelfits + _schema
deepCoadd_modelfits + _schema, deepCoadd_multiModelfits + _schema
diffsources + _schema
diffpsf
diff
crDiffim, crDiffimSrc + _schema
Datasets which are defined in obs_base may differ from those defined
in the Mapper.paf file only in the template or tables entries.
Remove those parts of the dataset definiton which do not differ.
@@ -750,11 +701,9 @@ datasets: {
# Measurements on coadds

# Reference catalog for coadds
# dataset defined in obs_base modified in this mapper as follows:
Copy link
Member Author

Choose a reason for hiding this comment

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

This python entry is definitely incorrect: it refers to the base class of the config class we want. I suspect obs_base has the right one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have this fix listed on RFC-237. Will do it next week.

}
# dataset defined in obs_base modified in this mapper as follows:
Copy link
Member Author

Choose a reason for hiding this comment

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

This python entry is incorrect. It should point to a class in meas.base, not pipe.tasks. Hopefully that change will bring it into agreement with obs_base.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have this fix listed on RFC-237. Will do it next week.

Copy link
Contributor

@PaulPrice PaulPrice left a comment

Choose a reason for hiding this comment

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

A few more minor comments.

storage: "FitsStorage"
level: "Skytile"
tables: "raw_skytile"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't leave two blank lines.

persistable: "Psf"
storage: "BoostStorage"
tables: "raw_skytile"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

storage: "FitsStorage"
level: "Skytile"
tables: "raw_skytile"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

persistable: "Psf"
storage: "BoostStorage"
tables: "raw_skytile"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

@@ -514,11 +514,9 @@ datasets: {
tables: "raw"
tables: "raw_visit"
}
# dataset defined in obs_base modified in this mapper as follows:
deepCoadd_calexp_background: {
template: "deepCoadd-results/%(filter)s/%(tract)d/%(patch)s/det_bkgd-%(filter)s-%(tract)d-%(patch)s.fits"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the HSC template different? Is there a reason not to just adopt the obs_base one?

@@ -527,17 +527,13 @@ datasets: {
tables: raw
tables: raw_visit
}
# dataset defined in obs_base modified in this mapper as follows:
forced_src_schema: {
template: "schema/forced_src.fits"
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, I think we should use what's in obs_base unless there's a good reason not to.

@pgee2000 pgee2000 merged commit 3933111 into master Oct 30, 2016
@ktlim ktlim deleted the tickets/DM-7884 branch August 25, 2018 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants