-
Notifications
You must be signed in to change notification settings - Fork 122
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
📚 todo: Specify if vacuum or air in crop function #582
Labels
todo
in the short-term roadmap
Comments
We'll have the same problem ("are nm in vacuum or in air by default ?") in every context, for instance in calc_spectrum input.
Based on this I would Make Air the default, unless stated otherwise in the Spectrum.
Eventually this would be the test : # The guiding principle is that .crop() and .plot() use the same waverange , so that it's very clear for the user
from radis import load_spec
from radis.test.utils import getTestFile
import astropy.units as u
#%% Work with a Spectrum object saved in nm_air
s2 = load_spec(getTestFile("N2C_specair_380nm.spec"), binary=False)
# Default crop (astropy units)
# ... we expect it happens in air
s_def = s2.crop(0.376 * u.um, 0.381 * u.um, inplace=False)
w_def = s_def.get_wavelength(medium="air")
# print(w_def.min(), w_def.max())
assert w_def.min() == 376
assert w_def.max() == 381
# Crop in air
s_air = s2.crop(376, 381, "nm_air", inplace=False)
w_air = s_air.get_wavelength(medium="air")
# print(w_def.min(), w_def.max())
assert w_air.min() == 376
assert w_air.max() == 381
# Crop in vacuum
s_vac = s2.crop(376, 381, "nm_vac", inplace=False)
w_vac = s_vac.get_wavelength(medium="vacuum")
# print(w_vac.min(), w_vac.max())
assert w_vac.min() == 376
assert w_vac.max() == 381
#%% Work with a Spectrum object saved in nm_vacuum
# WE DONT HAVE ANY AT THE MOMENT, HAVE WE ?
# else: adapt the visible spectrum above ; ex s3 = s2.resample(s2.get_wavelength(wunit='nm_vac'), unit='nm_vac') ??
#%% Work with Spectrum object saved in cm-1
s = load_spec(getTestFile("CO_Tgas1500K_mole_fraction0.01.spec"), binary=False)
# Default crop (astropy units)
# ... we expect it happens in air
s_def = s.crop(4.67 * u.um, 4.69 * u.um, inplace=False)
w_def = s_def.get_wavelength(medium="air")
# print(w_def.min(), w_def.max())
assert w_def.min() == 4670
assert w_def.max() == 4690
# Crop in air
s_air = s.crop(4670, 4690, "nm_air", inplace=False)
w_air = s_air.get_wavelength(medium="air")
# print(w_def.min(), w_def.max())
assert w_air.min() == 4670
assert w_air.max() == 4690
# Crop in vacuum
s_vac = s.crop(4670, 4690, "nm_vac", inplace=False)
w_vac = s_vac.get_wavelength(medium="vacuum")
# print(w_vac.min(), w_vac.max())
assert w_vac.min() == 4670
assert w_vac.max() == 4690
EDIT : Decided with @minouHub
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
💭 Description
In the
crop
function, if anastropy.unit
is fed as a waverange, there is an ambiguity if the wavelength is in air or vacuum. For instance:In this code, we do not know if the crop function will cut the spectrum in 'nm' or 'nm_air'. I suggest forcing the unit in the crop function and remove this astropy.units feature for this specific function. The use of crop would, in this case, be:
@erwanp I know you push for using
astropy.units
but I think it overcomplicates in this case.The text was updated successfully, but these errors were encountered: