-
Notifications
You must be signed in to change notification settings - Fork 19
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
Added script that solves satv1 pointing model parameters. #999
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks! Here's the round 1 review (there will be more rounds).
As a site_pipeline script, this should conform to site_pipeline standards (which I admit are not evenly enforced):
- Reformulate
main(...)
, add aget_parser()
function, and useutil.main_launcher
. Seesite_pipeline/preprocess_tod.py
for a correct example. - Register the function in
site_pipeline/cli.py
so it can be run from the command line usingso-site-pipeline
entry point. - Add an entry in docs/site_pipeline.rst. Make sure the text includes an example config file. If command line args are non-trivial, it should include those two. Both of those things can be "automagically" dropped in by sphinx, from the module code / docstrings, with the right constructions. (See other examples in site_pipeline.rst)
One more big request for overall scope: I know this fitter has been validated on this dataset, elsewhere. But when it is used on future data sets it will be essential to visualize the results, to see the residuals, and explore param combos. So... please include plotting functionality in here.
(Also a few minor comments inline.)
Added plotting functionality, added fit iteration, added documentation.
I had to do lots of the code editing on site computing while NERSC was down, so when I copied it back to NERSC to do the git push-ing, it makes it look like every line of code is new! Sorry |
It looks like the test docs build is failing because the code needs lmfit module to run. I'm not sure how to fix this issue. |
Includes more information on weights and residuals of outlying data points.
I couldn't get the proper formatting to have some of the lmfit parameter fits (i.e. correlation, stderr...etc) saved in the axis manager --> .h5 output file. So, I tried to make everything as clear as possible in the log file. For the outlier rejection, the log files show what data points are rejected, (obs_Id, Roll, El, Weight, and fit residual). (Some outliers have zero weight, and so don't affect fits). |
Script that solves for pointing model parameters given an input config and saves manifest/metadata.