Skip to content
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

LSST DESC Note: The Twinkles 1 Strong Lens Population #450

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

drphilmarshall
Copy link
Contributor

@jbkalmbach Here's an ipython notebook for us to write up the SL inputs work (showing the sprinkler in action, and the lens matching etc, #449). Can you take a look and maybe refine the section headings, do some subsectioning etc?

It looks to me as though we might need to refactor the sprinkler code so that it handles the lens galaxy matching - that stuff is just in an example notebook right now. What do you think?

We won't merge this PR until we've finished writing the Note and the group has reviewed it.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 41.35% when pulling f06cbfc on issue/449/twinkles1-sl-pop-note into e0da20a on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 41.35% when pulling d82eb00 on issue/449/twinkles1-sl-pop-note into e0da20a on master.

@jbkalmbach
Copy link
Member

The lens galaxy matching shouldn't be a part of the sprinkler since we just run the example notebook once and create a new fits catalog of lens systems with the additional information we have added about the lens galaxies. At that point the sprinkler just uses this new catalog whenever you run it. I think we should certainly mention it in the mock catalog section of the notebook and maybe even summarize the example notebook adding some of the plots from there, but I don't think the code belongs in the sprinkler.

@drphilmarshall
Copy link
Contributor Author

drphilmarshall commented May 24, 2017 via email

@coveralls
Copy link

Coverage Status

Coverage remained the same at 41.35% when pulling f179420 on issue/449/twinkles1-sl-pop-note into e0da20a on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 41.35% when pulling 5f5c664 on issue/449/twinkles1-sl-pop-note into e0da20a on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 41.35% when pulling f299cae on issue/449/twinkles1-sl-pop-note into e0da20a on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 41.35% when pulling d461539 on issue/449/twinkles1-sl-pop-note into e0da20a on master.

@jbkalmbach
Copy link
Member

This note is ready for comments. The Discussion and Conclusions section is kind of a set of bullet point response right now and any comments on the content there would be helpful before I write it out.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 41.35% when pulling cc5d7c6 on issue/449/twinkles1-sl-pop-note into e0da20a on master.

Copy link
Contributor Author

@drphilmarshall drphilmarshall left a comment

Choose a reason for hiding this comment

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

Thanks @jbkalmbach - looking at the main.ipynb notebook now. Some comments:

  • The JBK/PJM flags in the text cells need removing - except where you want me to make edits or add text.

  • I'm confused about the 2-mag offset in AGN image brightness - this seems hard to make just by getting the SED wrong (using rest frame instead of observed frame). Are you sure there is not some other problem, with the normlization / zero point? Are you comparing unmagnified source plane inputs with observed magnified outputs? 2 mags is a lot. Are all the source plane AGN too bright by this amount?

  • I don't see the matching work you did, with empiriciSN and XDGMM. Wasn't that in a separate notebook? I think it needs to be pasted in here to keep all your sprinkling work together.

  • It's good to see the quasar image positions line up with OM10!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants