Ticket #774 (closed defect: fixed)

Opened 10 years ago

Last modified 7 years ago

meas_pipeline: python coding error: local variable 'psfClumpIxx' referenced before assignment

Reported by: RayPlante Owned by:
Priority: critical Milestone:
Component: meas_algorithms Keywords:
Cc: ktl Blocked By:
Blocking: Project: LSST
Version Number:
How to repeat:

not applicable

Description

Occasionally, the stage 17 of the pipeline raises an exception apparently due to a programming error:

  File "/lsst/DC3/stacks/gcc412/24apr/Linux64/meas_algorithms/3.0.7/python/lsst/
meas/algorithms/Psf.py", line 139, in getClump
    if psfClumpIxx < IzzMin or psfClumpIyy < IzzMin:
UnboundLocalError: local variable 'psfClumpIxx' referenced before assignment

For more details, see /home/rplante/dc3pipe-exec/root/rlp1151/IPSD/work/Slice1.log

Change History

comment:1 Changed 10 years ago by ktl

This can only occur if no peaks are found in the Ixx/Iyy? histogram, which in turn is likely to only occur if there are no good PSF candidates.

comment:2 Changed 10 years ago by rhl

  • Status changed from new to assigned
  • Owner changed from rhl to RayPlante

As KT says, it probably failed to identify any PSF candidates. Please generate a SimpleStage script that reproduces this, and assign the ticket back to me.

Also, if there's useful information in the Slice logs please attach to the ticket, rather than pointing me to a file on a local NCSA disk that may not be there in a couple of months if this problem reoccurs. We should strive to make tickets self-contained (as the logs are large, it might be better to define a permanent well-defined location somewhere at NCSA that all files associated with a given ticket are stored). In this case I didn't see anything useful, in particular I didn't see where the exposure/CCD/amp information was stored, or which versions of meas_algorithms and meas_pipeline was employed (and the version isn't filled out in this ticket)

comment:3 Changed 10 years ago by ktl

This is v707187-e0-c000-a01. meas_algorithms 3.0.7, meas_pipeline svn9473.

comment:4 Changed 10 years ago by rowen

This may be obvious, but the error appears to be in python/lsst/meas/algorithms/Psf.py and it is clear that psfClumpIxx, psfClumpIxy and psfClumpIyy need not be defined before they are used. They are set in this loop:

        for i in range(len(objects)):
            source = afwDetection.Source()
            sourceList.append(source)
            source.setId(i)
            
            try:
                measureSources.apply(source, objects[i])
            except Exception, e:
                continue
            
            x, y = source.getXAstrom(), source.getYAstrom()
            val = mpsfImage.getImage().get(int(x), int(y))

            if Imax is None or val > Imax:
                Imax = val
                psfClumpX, psfClumpY = x, y
                psfClumpIxx = source.getIxx()
                psfClumpIxy = source.getIxy()
                psfClumpIyy = source.getIyy()

but they are not initialized to anything reasonable -- what should happen if no suitable sources are found?

comment:5 Changed 10 years ago by ktl

  • Owner changed from RayPlante to rhl

Attachments are limited to 2MB, so I can't upload the entire "fail" directory here. It can be found in /lsst/DC3root/ticketFiles/774/.

comment:6 Changed 10 years ago by rhl

  • Status changed from assigned to inTicketWork

I can reproduce this; thanks KT.

In response to Russell, the immediate problem is indeed that the variable isn't set, but the question is why? As part of the investigation I'll generate a more explicit error message.

comment:7 Changed 10 years ago by rhl

  • Status changed from inTicketWork to new
  • Owner rhl deleted

The problem is that this frame has horrible seeing, and the image used to find the commonest second moment wasn't large enough; consequently, an attempt to measure its shape failed with an exception that was caught and ignored.

For now, I've enlarged the array (allowing twice as bad seeing), and changed the error processing to throw a helpful exception. In the future, we should revisit how to handle this case more gracefully.

Fixed in meas_algorithms r9485

comment:8 Changed 10 years ago by rhl

  • Status changed from new to closed
  • Resolution set to fixed
  • reviewstatus changed from notReady to selfReviewed

comment:9 Changed 10 years ago by RayPlante

May I suggest that there is a programming error, announced by the UnboundLocalError? exception, that has not been addressed in this fix? It is an error to reference a variable before it is initialized. Given that the initialization happens within an if block, is it possible that the if expression might always be false and therefore repeat this error. I'm sure I'm missing some of the "business" logic here; however, might it be better to initialize the variables prior to entering the loop (say, with None) and testing for the possibility that it was not updated?

comment:10 Changed 10 years ago by rhl

Take a look at the patch. The variable Imax is set to None, and tested to see if it's been set --- the *Clump* variables are set iff it is.

comment:11 Changed 7 years ago by robyn

  • Milestone DC3a Completed deleted

Milestone DC3a Completed deleted

Note: See TracTickets for help on using tickets.