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

Empirical spec phot #14

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

Empirical spec phot #14

wants to merge 16 commits into from

Conversation

tcjansen
Copy link
Owner

A place for comments on this example!

The purpose of this example is to take a real spectrum and perform synthetic photometry on it with synphot.

skymodel.py Outdated Show resolved Hide resolved
skymodel.py Show resolved Hide resolved
@bmorris3
Copy link

This demo looks great so far! I look forward to working with you to get the predicted mags closer to the observed mags 😄

@@ -1 +0,0 @@
Browse the branches to see the different notebooks I've been working on!

Choose a reason for hiding this comment

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

It'd be great to have some more info in the readme – perhaps a link to your blog, plus a link to the projects page to point out your goals/progress?

skymodel.py Outdated Show resolved Hide resolved
@eteq
Copy link

eteq commented Jun 24, 2019

I said this on Slack but I figure I should have it here for completeness. Will try to do more of a review later, but some initially thoughts:

  • within 30% or so is a really good sign! And surely meets the baseline "telescopy" intent of something good enough for the "is my proposal in the right ballpark?" question.
  • I wonder if the mirror/instrument is the missing piece? I.e., is the SVO SDSS curve atmosphere + filter, or total throughput with everything included? That might show some of the difference
  • If the seeing is dramatically different between the spec and phot observations that might matter at this level. See the spectrophotometry and seeing pages from SDSS for more on that

skymodel.py Outdated Show resolved Hide resolved
skymodel.py Outdated Show resolved Hide resolved
skymodel.py Outdated Show resolved Hide resolved
Co-Authored-By: P. L. Lim <2090236+pllim@users.noreply.github.com>
@tcjansen
Copy link
Owner Author

I wonder if the mirror/instrument is the missing piece? I.e., is the SVO SDSS curve atmosphere + filter, or total throughput with everything included? That might show some of the difference

Quoting from the e-mail conversation @bmorris3 and I had with Carlos Rodrigo at SVO, "the [filter curves] that [SVO] provide are the ones including not only the filter transmission but
also the full system response from atmosphere to detector", so I don't think the error can be attributed to that unfortunately.

"fluxes = {}\n",
"for band in sdss_bands:\n",
" observation = Observation(spectrum, bandpasses[band], force='extrap')\n",
" flux = observation.effstim('Jy')\n",

Choose a reason for hiding this comment

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

It'd be great if you introduced effstim before you used it. When I first encountered it, for example, I wasn't sure what it abbreviated, and what calculation it was doing under-the-hood.

Copy link

Choose a reason for hiding this comment

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

# Delete the file after reading from it
os.remove(tmp_data_file)

return trans_waves.to(u.angstrom), transmission

Choose a reason for hiding this comment

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

Little style nitpick: Python files should end with a blank newline.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hmm I'm not sure why github isn't showing my last blank line, as they're definitely there in my text editor (sublime text)...

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.

None yet

4 participants