DM-14155 make jointcal compatible with other source selectors #83
Conversation
parejkoj
changed the title
parejkoj
force-pushed
the
tickets/DM-14155
branch
5 times, most recently
from
d24278b
to
8b9b43b
Compare
In addition to the sourceSelector API changes in DM-9832, jointcal wasn't compatible with other sourceSelectors anyway. This fixes both issues.
parejkoj
force-pushed
the
tickets/DM-14155
branch
from
8b9b43b
to
614a1b5
Compare
tests/test_jointcal_decam.py
Outdated
|
|
||
| return pa1, metrics | ||
|
|
||
| @unittest.skip("This test produces different chi2/ndof on Linux and macOS.") |
There was a problem hiding this comment.
If there is a ticket to investigate and fix this discrepancy, could you make a note of it here? And if there isn't one yet, I suggest you create one.
| """ | ||
| self.config = lsst.jointcal.jointcal.JointcalConfig() | ||
| self.config.astrometryRefObjLoader.retarget(LoadAstrometryNetObjectsTask) | ||
| self.config.photometryRefObjLoader.retarget(LoadAstrometryNetObjectsTask) |
There was a problem hiding this comment.
Should these both be LoadAstrometryNetObjectsTask?
There was a problem hiding this comment.
Yup. The reference catalog loader is the astrometry.net ObjectsLoader. Nothing to do with whether it's used for photometry or astrometry.
tests/test_jointcal_decam.py
Outdated
| self.jointcalStatistics.do_astrometry = False | ||
|
|
||
| pa1 = 0.11 | ||
| metrics = {'collected_photometry_refStars': 4865, |
There was a problem hiding this comment.
Some comment about where all these numbers come from would be helpful.
There was a problem hiding this comment.
They're all emperical, once I've gotten the code to actually work. That's true for all of the metrics blocks in jointcal's tests because I don't know the correct result a priori, except for the cfht_minimal test which was designed so that I could manually compute the correct answer.
I have a note in the readme about this: https://github.com/lsst/jointcal/blob/master/Readme.md
Do you think I should include a similar note along with each of these blocks?
There was a problem hiding this comment.
Since the answer is slightly involved, I'd suggest you reference the Readme, or include that disclaimer once at the top of the file and reference that in tests with empirical test values.
tests/test_jointcal_cfht.py
Outdated
| self.jointcalStatistics.do_astrometry = False | ||
|
|
||
| pa1 = 0.026 | ||
| metrics = {'collected_photometry_refStars': 825, |
There was a problem hiding this comment.
Some comment about where all these numbers come from would be helpful.
decam tests are skipped due to macOS/Linux mismatch. Sorting out that numeric mismatch is outside the scope of this ticket, but is part of the scope of figuring out why the constrainedPhotometryModel is not working, in general.
Added some more details about the test numbers to the readme, and referenced it in tests that have such numbers.
parejkoj
force-pushed
the
tickets/DM-14155
branch
from
614a1b5
to
2ce3d9d
Compare
This can't be merged until DM-9832 is finished.