Skip to content

Brighter Fatter Correction Tutorial (using beam sim data)#94

Merged
andrewkbradshaw merged 13 commits into
masterfrom
project/beamsim/andrewkbradshaw
Sep 14, 2018
Merged

Brighter Fatter Correction Tutorial (using beam sim data)#94
andrewkbradshaw merged 13 commits into
masterfrom
project/beamsim/andrewkbradshaw

Conversation

@andrewkbradshaw
Copy link
Copy Markdown
Member

This isn't finished, but I think it is almost ready for feedback. I just need to make sure it runs on the data in /project/shared/beamsim/ . Then, I think the main thing I will need is some review by someone from the DM team. Would that be you @SimonKrughoff ?

@SimonKrughoff
Copy link
Copy Markdown
Contributor

I can take an initial look, but someone at Princeton may be a better fit. I'll try to point the right person at it.

@drphilmarshall
Copy link
Copy Markdown
Contributor

Simon mentioned Josh Myers might like to review this tutorial - what do you think, @jmeyers314?

@drphilmarshall
Copy link
Copy Markdown
Contributor

I saw @jmeyers314 on the last day of the PCW, and he said he'd be happy to review @andrewkbradshaw 's beamsim B/F notebook. Assigning Josh as reviewer - thanks Josh! :-)

@andrewkbradshaw
Copy link
Copy Markdown
Member Author

I think I am finally done for now and it should be ready for a review @jmeyers314

Copy link
Copy Markdown

@jmeyers314 jmeyers314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

A few general comments.

• It looks like there are some spurious commits on this branch that probably belong elsewhere? Probably need to rebase this branch before merging it back to master.

• It would be nice to have some more context in places about why you're generating specific plots. It'd probably be nice to have these is dedicated markdown cells sprinkled throughout.

• Also, I think it'd be good to have a summary markdown cell at the bottom commenting on what you've learned from the various plots.

• (Finally, thanks to the rest of stack club for helping me review this!)

Comment thread ImageProcessing/BrighterFatterCorrection.ipynb Outdated
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb Outdated
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb
Comment thread ImageProcessing/BrighterFatterCorrection.ipynb
@andrewkbradshaw andrewkbradshaw merged commit 3da0d82 into master Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants