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

DM-5120: Add intelligence to validate_drp so it does "A Reasonable Thing" on an unknown output repo #10

Merged
merged 8 commits into from Mar 3, 2016

Conversation

wmwv
Copy link
Contributor

@wmwv wmwv commented Feb 20, 2016

validateDrp.py Cfht/output

will now do "something reasonable" with no further configuration information needed.

Return only dataIds with 'src' and 'calexp' on disk.

use queryMetadata() to retrieve the filter information
in case the dataId from Butler.Subset doesn't include it
Update README.md and examples for new calling format for `validateDrp.py`
calib = afwImage.Calib(butler.get("calexp_md", vId, immediate=True))
except TypeError as te:
print(te)
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would this throw a TypeError? (I'm not claiming it won't or can't, I'm just a little surprised.) If it happens, can we do something more informative than just printing the exception? (Even adding "Skipping to next data ID" might be useful, for e.g.).

Also, this code is added in 8fc82e0, which is fundamentally about a different issue (adding the no-throw decorator, below). Can we split it into its own commit with a message that explains why it's necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. Now documented in code comment below.

            # DECam images that haven't been properly reformatted
            # can trigger a TypeError because of a residual FITS header
            # LTV2 which is a float instead of the expected integer.
            # This generates an error of the form:
            #
            # lsst::pex::exceptions::TypeError: 'LTV2 has mismatched type'
            #
            # See, e.g., DM-2957 for details.

Also added catching FitsError exception when reading FITS files.

Copy link

Choose a reason for hiding this comment

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

Didn't DM-4133 solve the LTV1/2 mismatched type issue? Or is this from something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is from the same problem solved by DM-4133. Unfortunately problems in reduced data persist even after code has been updated. I think we'll unfortunately be seeing such issues and accumulating a bit of cruft like this as we have to deal with more and more actual data.

When I ran validateDrp.py on the DECam COSMOS data as processed as NCSA, there were still a number of files with this issue. This catch is designed to skip those and proceed onward.

MWV

On Mar 3, 2016, at 10:29, hchiang2 notifications@github.com wrote:

In python/lsst/validate/drp/validate.py:

@@ -88,17 +92,23 @@ def loadAndMatchData(repo, visitDataIds,
srcVis = SourceCatalog(newSchema)

 for vId in visitDataIds:
  •    calib = afwImage.Calib(butler.get("calexp_md", vId, immediate=True))
    
  •    calib.setThrowOnNegativeFlux(False)
    
  •    try:
    
  •        calib = afwImage.Calib(butler.get("calexp_md", vId, immediate=True))
    
  •    except TypeError as te:
    
  •        print(te)
    
  •        continue
    

Didn't DM-4133 solve the LTV1/2 mismatched type issue? Or is this from something else?


Reply to this email directly or view it on GitHub.

If the configFile has only validation parameters specified (e.g., good_mag_limit)
then use util.discoverDataIds to identify the dataIds to analyze.

Update naming of visitDataIds->dataIds.
wmwv added 5 commits March 2, 2016 22:52
Use afwImageUtils.CalibNoThrow context manager instead of
explicitly setting the state in the Calib object

Add catching FitsError + explain TypeError reading calexp

Clarify error messages from missing calibration.
Explain why the TypeError needs to be caught when reading
LTV2 keywords in DECam images that haven't been fully sanitized.
The caveat is that this is single-node, load-everything-in-memory,
and then match.  This is infeasible at scale.  Specifically,
this currently fails on lsst-dev at more than ~500 catalogs in a band.
@wmwv wmwv merged commit 7e65310 into master Mar 3, 2016
@ktlim ktlim deleted the tickets/DM-5120 branch August 25, 2018 06:50
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.

None yet

3 participants