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

fix slow jointcal output writing Tickets/DM-8978 #25

Merged
merged 1 commit into from
Jan 14, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 39 additions & 30 deletions python/lsst/jointcal/jointcal.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,20 @@ def _build_ccdImage(self, dataRef, associations, jointcalControl):

Returns
------
afw.image.TanWcs
the TAN WCS of this image
namedtuple
wcs : lsst.afw.image.TanWcs
the TAN WCS of this image, constructed from the calexp_md
key : namedtuple
a key to identify this dataRef by its visit and ccd ids
Copy link

Choose a reason for hiding this comment

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

Might be useful to make the key a namedtuple with 'visitId' and 'ccdId' names.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was going to argue that the key should be an opaque object, but I can see it being quite useful to have the parts named (may need to add "tract" or "filter" in the future, anyway). Done.

"""
visit = dataRef.dataId["visit"]
src = dataRef.get("src", immediate=True)
md = dataRef.get("calexp_md", immediate=True)
calexp = dataRef.get("calexp", immediate=True)
visitInfo = calexp.getInfo().getVisitInfo()
ccdname = calexp.getDetector().getId()

tanwcs = afwImage.TanWcs.cast(afwImage.makeWcs(md))
tanWcs = afwImage.TanWcs.cast(afwImage.makeWcs(md))
lLeft = afwImage.getImageXY0FromMetadata(afwImage.wcsNameForXY0, md)
uRight = afwGeom.Point2I(lLeft.getX() + md.get("NAXIS1") - 1, lLeft.getY() + md.get("NAXIS2") - 1)
bbox = afwGeom.Box2I(lLeft, uRight)
Expand All @@ -176,14 +180,15 @@ def _build_ccdImage(self, dataRef, associations, jointcalControl):
goodSrc = self.sourceSelector.selectSources(src)

if len(goodSrc.sourceCat) == 0:
print("no stars selected in ", dataRef.dataId["visit"], ccdname)
return tanwcs
print("%d stars selected in visit %d - ccd %d" % (len(goodSrc.sourceCat),
dataRef.dataId["visit"],
ccdname))
associations.AddImage(goodSrc.sourceCat, tanwcs, visitInfo, bbox, filt, calib,
dataRef.dataId['visit'], ccdname, jointcalControl)
return tanwcs
print("no stars selected in ", visit, ccdname)
return tanWcs
print("%d stars selected in visit %d - ccd %d" % (len(goodSrc.sourceCat), visit, ccdname))
associations.AddImage(goodSrc.sourceCat, tanWcs, visitInfo, bbox, filt, calib,
visit, ccdname, jointcalControl)

Result = collections.namedtuple('Result_from_build_CcdImage', ('wcs', 'key'))
Key = collections.namedtuple('Key', ('visit', 'ccd'))
return Result(tanWcs, Key(visit, ccdname))

@pipeBase.timeMethod
def run(self, dataRefs, profile_jointcal=False):
Expand Down Expand Up @@ -212,8 +217,13 @@ def run(self, dataRefs, profile_jointcal=False):
associations = jointcalLib.Associations()

load_cat_prof_file = 'jointcal_load_catalog.prof' if profile_jointcal else ''
visit_ccd_to_dataRef = {}
oldWcsList = []
with pipeBase.cmdLineTask.profile(load_cat_prof_file):
oldWcsList = [self._build_ccdImage(ref, associations, jointcalControl) for ref in dataRefs]
for ref in dataRefs:
result = self._build_ccdImage(ref, associations, jointcalControl)
oldWcsList.append(result.wcs)
visit_ccd_to_dataRef[result.key] = ref
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked at the code in detail, but is there a reason not to use result.Name() here and avoid having to parse it below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm confused? result is a namedtuple, with "wcs" and "key", there's no "Name". I don't know what you're referring to with this comment.


matchCut = 3.0
# TODO: this should not print "trying to invert a singular transformation:"
Expand Down Expand Up @@ -271,7 +281,7 @@ def run(self, dataRefs, profile_jointcal=False):
tupleName = "res_" + str(dataRefs[0].dataId["tract"]) + ".list"
astrometry.fit.makeResTuple(tupleName)

self._write_results(associations, astrometry.model, photometry.model, dataRefs)
self._write_results(associations, astrometry.model, photometry.model, visit_ccd_to_dataRef)

return pipeBase.Struct(dataRefs=dataRefs, oldWcsList=oldWcsList)

Expand Down Expand Up @@ -372,7 +382,7 @@ def _fit_astrometry(self, associations):
Astrometry = collections.namedtuple('Astrometry', ('fit', 'model', 'sky_to_tan_projection'))
return Astrometry(fit, model, sky_to_tan_projection)

def _write_results(self, associations, astrom_model, photom_model, dataRefs):
def _write_results(self, associations, astrom_model, photom_model, visit_ccd_to_dataRef):
"""
Write the fitted results (photometric and astrometric) to a new 'wcs' dataRef.

Expand All @@ -384,8 +394,8 @@ def _write_results(self, associations, astrom_model, photom_model, dataRefs):
The astrometric model that was fit.
photom_model : lsst.jointcal.PhotomModel
The photometric model that was fit.
dataRefs : list of lsst.daf.persistence.ButlerDataRef
List of data references to the exposures that were fit.
visit_ccd_to_dataRef : dict of Key: lsst.daf.persistence.ButlerDataRef
dict of ccdImage identifiers to dataRefs that were fit
"""

ccdImageList = associations.getCcdImageList()
Expand All @@ -397,17 +407,16 @@ def _write_results(self, associations, astrom_model, photom_model, dataRefs):
# TODO: there must be a better way to identify this ccdImage?
name = ccdImage.Name()
visit, ccd = name.split('_')
for dataRef in dataRefs:
calexp = dataRef.get("calexp")
ccdname = calexp.getDetector().getId()
if dataRef.dataId["visit"] == int(visit) and ccdname == int(ccd):
print("Updating WCS for visit: %d, ccd%d" % (int(visit), int(ccd)))
exp = afwImage.ExposureI(0, 0, tanWcs)
exp.setCalib(calexp.getCalib()) # start with the original calib
fluxMag0, fluxMag0Sigma = calexp.getCalib().getFluxMag0()
exp.getCalib().setFluxMag0(fluxMag0*photom_model.photomFactor(ccdImage), fluxMag0Sigma)
try:
dataRef.put(exp, 'wcs')
except pexExceptions.Exception as e:
self.log.warn('Failed to write updated Wcs: ' + str(e))
break
visit = int(visit)
ccd = int(ccd)
dataRef = visit_ccd_to_dataRef[(visit, ccd)]
calexp = dataRef.get("calexp")
Copy link
Contributor

Choose a reason for hiding this comment

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

You could get "calexp_md" and use that to create the Calib for more time savings.

Copy link
Collaborator Author

@parejkoj parejkoj Jan 13, 2017

Choose a reason for hiding this comment

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

I would much rather remove the building of the tanwcs from the calexp_md and just get it from ExposureInfo (the tanWcs code there is original to meas_simastrom, and I haven't taken the time to replace it yet). Why should I touch calexp_md, if we've got pre-built objects in ExposureInfo?

print("Updating WCS for visit: %d, ccd: %d" % (visit, ccd))
exp = afwImage.ExposureI(0, 0, tanWcs)
exp.setCalib(calexp.getCalib()) # start with the original calib
fluxMag0, fluxMag0Sigma = calexp.getCalib().getFluxMag0()
exp.getCalib().setFluxMag0(fluxMag0*photom_model.photomFactor(ccdImage), fluxMag0Sigma)
try:
dataRef.put(exp, 'wcs')
except pexExceptions.Exception as e:
self.log.warn('Failed to write updated Wcs and Calib: ' + str(e))