Skip to content

DM-50420: Inject variable sources into the template and science images#1313

Open
BrunoSanchez wants to merge 1 commit into
mainfrom
tickets/DM-50420
Open

DM-50420: Inject variable sources into the template and science images#1313
BrunoSanchez wants to merge 1 commit into
mainfrom
tickets/DM-50420

Conversation

@BrunoSanchez
Copy link
Copy Markdown
Member

Fix matching to account for template fakes and mutual match strategy

@BrunoSanchez BrunoSanchez changed the title DM-50420 DM-50420: Inject variable sources into the template and science images Jun 1, 2026
Copy link
Copy Markdown
Contributor

@isullivan isullivan left a comment

Choose a reason for hiding this comment

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

A number of minor changes, but please think about whether the injectedCat->injectionCat rename is necessary.

ConfigClass = MatchInjectedToDiaSourceConfig

def run(self, injectedCat, diffIm, diaSources):
# def run(self, injectedCat, injectedTemplateCat, diffIm, diaSources):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove commented-out code

if column.endswith("instFlux"):
flux = injectedCat[column]
fluxErr = injectedCat[column+"Err"].copy()
# flux = injectionCat[column]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove commented-out code

"""
# First match the diaSrc to the injected fakes
injectedCat = injectedCat.to_pandas()
# injectedCat = injectedCat.to_pandas()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove commented-out code

Comment on lines +274 to +275
idxsAux = np.where(np.array(idxs) < len(diaSources), idxs, -1)
idxsBackMatched = np.where(idxsAux >= 0, idxsBack[idxsAux], -1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
idxsAux = np.where(np.array(idxs) < len(diaSources), idxs, -1)
idxsBackMatched = np.where(idxsAux >= 0, idxsBack[idxsAux], -1)
valid = idxsAux >= 0
idxsBackMatched = np.full_like(idxsAux, -1)
idxsBackMatched[valid] = idxsBack[idxsAux[valid]]

To better avoid edge cases when idxsAux=-1.

variableDoublesFakeCat['dist_diaSrc'] = -1

# Match variable sources to their twin's matched diaSource
for i, row in enumerate(variableDoublesFakeCat):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I worry that this loop over the fake sources will get very slow for large numbers of injected fakes. Could you use astropy.table.join?

Comment on lines -46 to +47
injectedCat = connTypes.Input(
injectionCat = connTypes.Input(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This connection name change breaks previous pipelines and tests. Is it necessary? If so, you'll need to update drp_pipe as well.

injectionCat = connTypes.Input(
doc="Catalog of sources injected in the images.",
name="{fakesType}_pvi_catalog",
name="{fakesType}VisitDetectorFakeSourceCat",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should {fakesType} be dropped from all of the connections?


def run(self, injectedCat, diffIm, diaSources):
# def run(self, injectedCat, injectedTemplateCat, diffIm, diaSources):
def run(self, injectionCat, diffIm, diaSources):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See above comment on injectedCat vs injectionCat


Parameters
----------
fakeCat : `astropy.table.table.Table`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One too many .table I think. Below as well.

"""
# Match the fakes to the associated sources. For this we don't use the coordinates
# but instead check for the diaSources. Since they were present in the table already
if not isinstance(assocDiaSources, Table):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this necessary, now that the connection is being read as Astropy?

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.

2 participants