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

esheldon fork Tickets/dm 40513 #24

Draft
wants to merge 19 commits into
base: tickets/DM-40513
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions python/lsst/drp/tasks/metadetection_shear.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,16 @@ class MetadetectionShearConnections(PipelineTaskConnections, dimensions={"patch"
dimensions={"patch", "band"},
)

# from Arun K.: The "name" field is configurable from the pipeline yaml
# file which are specific to what repo we are running this against.
# cal_ref_cat_2_2 is just the default value in the absence of a pipeline
# file
ref_cat = cT.PrerequisiteInput(
doc="Reference catalog used to mask bright objects.",
name="ref_cat",
# when using /repo/main
# name="gaia_dr3_20230707",
# when using /repo/dc2
name="cal_ref_cat_2_2",
storageClass="SimpleCatalog",
dimensions=("skypix",),
deferLoad=True,
Expand Down Expand Up @@ -141,9 +148,7 @@ class MetadetectionShearConfig(
"Bands expected to be present. Cells with one or more of these bands "
"missing will be skipped. Bands other than those listed here will "
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this docstring is consistent with what happens. If g is specified by the coadd data doesn't exist, I think a NoWorkFound gets raised and nothing gets processed. I think that's the correct behavior and the docstring needs to reflect that.

Copy link
Author

Choose a reason for hiding this comment

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

Jim wrote that doc string, maybe he can comment

Copy link
Member

Choose a reason for hiding this comment

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

I wrote what I expect to happen. Happy to have it changed to what will happen.

"not be processed.",
# TODO learn how to set in a config file
default=["g", "r", "i", "z"],
# default=["r"],
optional=False,
)

Expand All @@ -156,6 +161,13 @@ class MetadetectionShearConfig(

# TODO: expose more configuration options here.

def setDefaults(self):
super().setDefaults()
# This is a DC2/cal_ref_cat_2_2 specific hack. This should be ideally
# specified in a config file
# To be removed in the cleanup before merging to main
self.ref_loader.filterMap = {band: f"lsst_{band}_smeared" for band in self.required_bands}
Copy link
Member

Choose a reason for hiding this comment

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

This is actually not a hack as the comment above says. What I meant is that the mapping here is specific to the reference catalog loaded.



class MetadetectionShearTask(PipelineTask):
"""A PipelineTask that measures shear using metadetection."""
Expand Down Expand Up @@ -559,7 +571,8 @@ def runQuantum(
config=self.config.ref_loader,
log=self.log,
)
ref_cat = ref_loader.loadRegion(qc.quantum.dataId.region)
# What should decide the filterName?
ref_cat = ref_loader.loadRegion(qc.quantum.dataId.region, filterName="r") # THIS IS A HACK.
Copy link
Member

Choose a reason for hiding this comment

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

I know it says it's a hack but can't you get the band from the dataId?

Copy link
Member

@arunkannawadi arunkannawadi Oct 30, 2023

Choose a reason for hiding this comment

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

This is a multi-band task, so qc.quantum.dataId doesn't have a band.

Copy link
Member

@timj timj Oct 30, 2023

Choose a reason for hiding this comment

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

You do know all possible bands in play though because you calculate them in the next line. I have no idea how you guess which of the dataset ref bands is the one that you choose for the refcat loader though.

Copy link
Member

Choose a reason for hiding this comment

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

Exactly. I don't know either. And it might not matter at all for this purpose if the downstream code (which is metadetect here) does not rely on a reference band for fluxes (which I don't think it does)

Copy link
Author

Choose a reason for hiding this comment

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

correct, it does not


coadds_by_band = {
ref.dataId["band"]: qc.get(ref) for ref in inputRefs.input_coadds
Expand Down Expand Up @@ -620,7 +633,6 @@ def run(

single_cell_tables.append(table)
idstart += len(res)
break

# TODO: if we need to do any cell-overlap-region deduplication here
# (instead of purely in science analysis code), this is where'd it'd
Expand Down Expand Up @@ -942,7 +954,6 @@ def _simulate_coadd(rng):
)
from descwl_shear_sims.galaxies import make_galaxy_catalog
from descwl_shear_sims.psfs import make_fixed_psf, make_ps_psf, make_rand_psf

# from descwl_shear_sims.stars import make_star_catalog
from descwl_coadd.coadd_nowarp import make_coadd_nowarp
from metadetect.masking import get_ap_range
Expand Down
1 change: 1 addition & 0 deletions ups/drp_tasks.table
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ setupRequired(pipe_base)
setupRequired(utils)
setupRequired(meas_algorithms)
setupRequired(pipe_tasks)
setupRequired(metadetect)

# The following is boilerplate for all packages.
# See https://dmtn-001.lsst.io for details on LSST_LIBRARY_PATH.
Expand Down