-
Notifications
You must be signed in to change notification settings - Fork 0
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
Analysis notebook for tie vs danish #9
base: develop
Are you sure you want to change the base?
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
fd0151d
to
4fb7992
Compare
@@ -0,0 +1,344 @@ | |||
{ |
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.
Given that the ts_aos_analysis
notebook does not have outputs, it may be a good idea ( beyond attaching PDF of the executed notebook in the PR, and in the ticket) to describe the result here in words (eg. for Z7 Danish result of .. and TIE of ... with estimated uncertainty... agree with manual results ....) . But it is hard to describe results without any pictures, and with fleeting persistency of repos / collections / packages, I wonder if we shouldn't have a repo where these notebooks are actually executed.. (ts_aos_analysis_exec
) ?
Perhaps rewrite to avoid tautology, eg. Note we employ means of combined Zernike estimates from each of the 9 detectors
Reply via ReviewNB
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.
Bryce pointed out that we can't post executed notebooks anywhere public because many of the them contain embargoed data (hence why he deleted the PDFs in the PRs). We have created #aos-notebook-review on Slack to make the review process easier, but I agree we might want somewhere to store the executed notebooks. A private Github repo could work, but the size of the history will quickly get cumbersome. Maybe instead we create a shared Google drive folder where we can mirror the directory structure of the repo and drop PDFs of executed notebooks? It's a little cumbersome, but we could make adding the PDF to the drive be the last stage of the PR process?
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.
Yes, the slack channel is not a bad idea, but it may end up requiring too much scrolling with "v1", "better v1", "better v1 after comments" .... If we can create a shared private google drive (or some other approved location) that would work better (in reply to review one can just overwrite).
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 think the place to have that is probably JIRA in the different tickets, also for traceability in the future for the project, it would be better than an ad-hoc solution that only AOS people can access
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.
@gmegh we cannot put compiled PDFs in tickets because they frequently contain on-sky pixels
Notebook PDF here: https://rubin-obs.slack.com/archives/C07TYETT29J/p1730160158099849 |
Updated notebook PDF: https://rubin-obs.slack.com/archives/C07TYETT29J/p1730229784783939 |
@@ -0,0 +1,382 @@ | |||
{ |
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.
typo "mostly gree".
And I think it strengthens the text by putting a numerical value ("noisiest, with 150nm rms"), and later about disagreements (~200 nm difference for Z5, with amplitude of ~1200 nm is ~16% , and for Z13,Z20...)
Reply via ReviewNB
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.
Done
No description provided.