Conversation
Co-Authored-By: P. L. Lim <2090236+pllim@users.noreply.github.com>
… out binset and forced tapering in Observation calls
| @@ -1 +0,0 @@ | |||
| Browse the branches to see the different notebooks I've been working on! No newline at end of file | |||
There was a problem hiding this comment.
Why do I seem to see this diff in multiple PRs? Is this intentional?
There was a problem hiding this comment.
Ah, that's because I don't copy the README over to the new branches
|
Copying some questions from Slack:
I decided not to put it there because I couldn't figure out how to get
Nope - |
synspec_examples.ipynb
Outdated
| "name": "stderr", | ||
| "output_type": "stream", | ||
| "text": [ | ||
| "WARNING: W35: None:5:0: W35: 'value' attribute required for INFO elements [astropy.io.votable.tree]\n", |
There was a problem hiding this comment.
These warnings show disappear if you use yet-to-be-released astropy 4.0 😬 . In the meantime, if they bother you, you can filter them out with https://docs.python.org/3/library/warnings.html#overriding-the-default-filter .
| binset = np.array([first_wl]) | ||
| binwidth = first_wl / R | ||
|
|
||
| while binset[-1] <= waverange[-1].to(u.angstrom).value: |
There was a problem hiding this comment.
I haven't though very hard about this yet, but is there a way to write this without a while loop, to keep it speedy?
There was a problem hiding this comment.
Will think more about this!
synspec.py
Outdated
| Returns a spectrum with bins defined by a constant bin width specified | ||
| by the user. | ||
| """ | ||
| if not waverange: |
There was a problem hiding this comment.
I think it's a little clearer what you mean if you write this as if waverange is None:
| import numpy as np | ||
|
|
||
|
|
||
| def from_R(source_spectrum, bandpass, R, waverange, force='none'): |
There was a problem hiding this comment.
R is also another high level programming language. And typically uppercase variable signifies a constant, so I propose renaming R to resolution, spectral_resolution, or something like that.
| binset = np.array([first_wl]) | ||
| binwidth = first_wl / R | ||
|
|
||
| while binset[-1] <= waverange[-1].to(u.angstrom).value: |
There was a problem hiding this comment.
First of all, I don't think this can be vectorized easily as binset[i] depends on binset[i - 1]; and also we don't have to worry about speeding it up until it is proven to be a bottleneck; and even then Cythonizing it might be good enough.
However, I am concerned that this algorithm does not round-trip. For example:
waverange = [5000, 5500] * u.AA
R = 100 Following the math, I think binset here is actually bin edges. And I think you mean in your docstring that you want the bin's central wavelength divided by its bin width is R, correct? So, having calculated binset with this algorithm, my attempt to round trip as follows:
dw = binset[1:] - binset[:-1] # Bin widths
wcen = binset[:-1] + dw / 2 # Recalculate bin centers
observed_R = wcen / dwAnd I get 200.5, not 200.
So then, I tried to calculate the bins by hand, knowing the starting wavelength and R (knowing ending wavelength does not help much at the start here, but it lets you know when to stop the bins). However, I got stuck after the first bin. This is because, given that every bin is defined as wcen / dw = R, and I know the edge of the first bin, for the second bin, I know neither the wcen nor its dw in advance, and it seems to me that I have to solve for two unknown at once. Am I going crazy here?
Then I looked around in specutils and only found spectral_resolution() that calculates the R given the wavelengths, but not the other way around. This makes me think that maybe requiring R to be constant throughout is not practical in the first place?
🤔
| return Observation(source_spectrum, bandpass, binset=binset, force=force) | ||
|
|
||
|
|
||
| def from_wave_array(source_spectrum, bandpass, waveset, force='none'): |
There was a problem hiding this comment.
I don't see from_wave_array being used in the notebook, is this function necessary?
A place for comments on "synspec" - simulating spectroscopy with synphot tools.