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-29542: Move jointcal cfht tests to gen3 #205
Conversation
metrics['astrometry_final_ndof'] = 1644 | ||
configOptions, metrics = self.setup_jointcalTask_2_visits_constrainedAstrometry() | ||
metrics['astrometry_final_chi2'] = 1095.70 | ||
metrics['astrometry_final_ndof'] = 1740 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do these numbers come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the note in the package readme about this.
In this case, the input data changed (several years newer pipeline version), so the exact numbers changed a bit.
tests/test_astrometryModel.py
Outdated
import lsst.jointcal | ||
from lsst.jointcal.jointcal import make_schema_table, get_sourceTable_visit_columns, \ | ||
extract_detector_catalog_from_visit_catalog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought there was some preference for using from blah import (blah, blah, blah) when they went over a line but having read a bunch of things I can't find it. I'm adding this comment in case it triggers either of us to remember where I got this instruction in the past.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In middleware code we do use parentheses for multi-line imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see it mentioned in the dev guide anywhere either, but that's reasonable.
python/lsst/jointcal/jointcal.py
Outdated
@@ -910,6 +911,9 @@ def _extract_detector_catalog_from_visit_catalog(self, table, visitCatalog, dete | |||
|
|||
catalog = lsst.afw.table.SourceCatalog(table) | |||
matched = visitCatalog[detectorColumn] == detectorId | |||
n = sum(matched) | |||
if n == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if somehow the detectorIds all end up 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matched
here is a bool array, so sum(matched)
gives the number of elements that matched that detectorId
. If they're all 0, then sum(matched)
will be non-zero.
It turned out to be unnecessary for debugging a spurious error, but I think this is a useful thing to log to compare with the refcat region. Also improve the Frame string output.
* The HSC test data is still using the old field names. * Add gen3 test config with new field names
* Refactor to make importRepository a free function for use elsewhere. * Handle both CFHT refcats and HSC input collections
The dist_rms and PA1 tests are very embedded in gen2, and I will remove them entirely once I finish converting DECam and HSC to gen3, since they aren't really relevant anymore now that we are running more robust analysis in faro and analysis_drp
FlaggedSourceSelector hasn't been updated to work with the new gen3 visit-level catalogs.
8f4b403
to
4d6e6bc
Compare
Refactor sourceTable handling to be usable in tests: Jointcal uses sourceTable_visit files in gen3, which need to have the individual detector records pulled out when creating each CcdImage. Pulling these out to be free functions lets me reuse them in tests that read the files independently of JointcalTask. Update test_astrometryModel for gen3 cfht data: * Some other cleanups to variable/method names. * Had to increase some tolerance levels: the new cfht data has different WCSs, so the initialized Jointcal WCS will be different, so the inverse WCS tests might change a little. Update test_jointcal for refactored methods
4d6e6bc
to
11a1512
Compare
No description provided.