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.

Sign up for GitHub

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

Jump to bottom

DM-43389: Add option to disable loading diaForcedSource history #197

Merged
merged 1 commit into from
Mar 20, 2024
Merged

DM-43389: Add option to disable loading diaForcedSource history #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 11 additions & 1 deletion python/lsst/ap/association/diaPipe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,15 @@ class DiaPipelineConfig(pipeBase.PipelineTaskConfig,
target=DiaObjectCalculationTask,
doc="Task to compute summary statistics for DiaObjects.",
)
doLoadForcedSources = pexConfig.Field(
dtype=bool,
default=True,
deprecated="Added to allow disabling forced sources for performance"
"reasons during the ops rehearsal."
" It is expected to be removed.",
doc="Load forced DiaSource history from the APDB?"
"This should only be turned off for debugging purposes.",
)
Comment on lines +253 to +255
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this docstring should say something about how this was added to let us turn off forced sources for performance reasons? Maybe something like this?

Suggested change
doc="Load forced DiaSource history from the APDB?"
"This should only be turned off for debugging purposes.",
)
doc="Load forced DiaSource history from the APDB?"
" This should only be turned off for debugging purposes."
" Added to allow disabling forced sources for performance reasons during the ops rehearsal.",
)

diaForcedSource = pexConfig.ConfigurableField(
target=DiaForcedSourceTask,
doc="Task used for force photometer DiaObject locations in direct and "
Expand Down Expand Up @@ -370,7 +379,8 @@ def run(self,
DiaSources. (`pandas.DataFrame`)
"""
# Load the DiaObjects and DiaSource history.
loaderResult = self.diaCatalogLoader.run(diffIm, self.apdb)
loaderResult = self.diaCatalogLoader.run(diffIm, self.apdb,
doLoadForcedSources=self.config.doLoadForcedSources)
if len(loaderResult.diaObjects) > 0:
diaObjects = self.purgeDiaObjects(diffIm.getBBox(), diffIm.getWcs(), loaderResult.diaObjects,
buffer=self.config.imagePixelMargin)
Expand Down
19 changes: 17 additions & 2 deletions python/lsst/ap/association/loadDiaCatalogs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def __init__(self, **kwargs):
pipeBase.Task.__init__(self, **kwargs)

@timeMethod
def run(self, exposure, apdb):
def run(self, exposure, apdb, doLoadForcedSources=True):
"""Preload all DiaObjects and DiaSources from the Apdb given the
current exposure.

Expand All @@ -66,6 +66,11 @@ def run(self, exposure, apdb):
An exposure with a bounding box.
apdb : `lsst.dax.apdb.Apdb`
AP database connection object.
doLoadForcedSources : `bool`, optional
Load forced DiaSource history from the APDB?
This should only be turned off for debugging purposes.
Comment on lines +70 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

Same note as above with the config docstring.

Added to allow disabling forced sources for performance
reasons during the ops rehearsal.

Returns
-------
Expand All @@ -79,6 +84,13 @@ def run(self, exposure, apdb):
exposure padded by ``pixelMargin``. DataFrame is indexed by
``diaObjectId``, ``band``, ``diaSourceId`` columns.
(`pandas.DataFrame`)
- ``diaForcedSources`` : Complete set of forced photometered fluxes
on the past 12 months of difference images at DiaObject locations.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Returns block above this is missing diaForcedSources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha. That's been missing for a while... I'll add it.

Raises
------
RuntimeError
Raised if the Database query failed to load DiaObjects.
"""
region = self._getRegion(exposure)

Expand All @@ -95,7 +107,10 @@ def run(self, exposure, apdb):

diaSources = self.loadDiaSources(diaObjects, region, dateTime, apdb)

diaForcedSources = self.loadDiaForcedSources(diaObjects, region, dateTime, apdb)
if doLoadForcedSources:
diaForcedSources = self.loadDiaForcedSources(diaObjects, region, dateTime, apdb)
else:
diaForcedSources = pd.DataFrame(columns=["diaObjectId", "diaForcedSourceId"])

return pipeBase.Struct(
diaObjects=diaObjects,
Expand Down
Loading