-
Notifications
You must be signed in to change notification settings - Fork 8
DM-17256: Add AAS astrometric residual plotting code to validate_drp #98
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
base: main
Are you sure you want to change the base?
Conversation
These scripts were originally created to make plots for John Parejko's AAS 233 poster about jointcal. The "-from_pickle" one was written to play with plot options on the output of the other, since generating the data is slows. Added support for doing calculations and plots on the focalplane
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 suggest that this code should live in
pipe_analysis.
https://github.com/lsst-dm/pipe_analysis - Separate more clearly into a parse method and a run method.
- The read-from pickle files script should be reworked and greatly simplified to accept a list of files and just iterate through those and call the run method of
plot_astrometric_residuals.
| return goodMatches, averageCoord, distance | ||
|
|
||
|
|
||
| def count_ccds(goodMatches): |
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.
count_objects_on_ccds
would be a more descriptive name.
| name = 'single' | ||
| # name = 'jointcal' | ||
| # the path where the pickle/ directory is | ||
| path = '~/lsst/temp/AAS2019/quiver' |
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.
This script seems useful, but all of this hard coding makes it much less appropriate for general inclusion in a LSST package.
Rework this script so it accepts these arguments (the pick filename(s)) and options ('single', 'jointcal') from the command line.
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.
And really there should be a function that takes the in-memory objects, and then a wrapper that can be used to load those objects from a file.
| @@ -0,0 +1,77 @@ | |||
| #!/usr/bin/env python | |||
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.
Add LICENSE text block.
| @@ -0,0 +1,281 @@ | |||
| #!/usr/bin/env python | |||
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.
Add LICENSE text block.
| import lsst.meas.astrom | ||
| import lsst.pex.config | ||
| import lsst.pipe.base | ||
| from lsst.validate.drp.util import positionRmsFromCat, averageRaDecFromCat |
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.
Go ahead and copy these routines locally to remove the dependency on the validate_drp package. A little code duplication to avoid a chain of dependencies is definitely worth it in this case.
|
|
||
|
|
||
| def count_ccds(goodMatches): | ||
| """Count how many objects are on each ccd.""" |
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.
Describe what it returns, which is a dict of ccd: count.
| # but I can't be bothered to do the "getDetector()" stuff, and you can't | ||
| # get at a visit ID in any other way anyway... | ||
| visit = dataRef.dataId['visit'] | ||
| ccd = dataRef.dataId['ccd'] |
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.
How robust will this be on other cameras where the unique int for each sensor might be called 'ccd' or 'detector' or 'ccdname', etc.?
|
|
||
| xlim = (bbox.getMinX(), bbox.getMaxX()) | ||
| ylim = (bbox.getMinY(), bbox.getMaxY()) | ||
| filename = "pickles/quiverData-%s-%s.pickle"%(name, ccd) |
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.
Define format template outside of for loop. Consider making the format template a keyword-option to this function.
| plt.close(fig) | ||
|
|
||
|
|
||
| def main(): |
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 would call this run instead of main.
|
|
||
| if len(args.id.refList) == 0: | ||
| raise RuntimeError("No data to process! Check your id list and/or input data path.") | ||
| fluxField, newSchema, mapper, multiMatch = prep_matching(butler, args.id.refList[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.
Separate this function here where the stuff above is parse
and the lines below are run.
No description provided.