wstep='auto' fails for multiple molecules

erwanp commented 2 years ago

🐛 Describe the bug

'auto' mode doesn't work with multiple molecules. Crashes Radis-app https://github.com/suzil/radis-app/issues/463

🖥 Steps To Reproduce

from radis import calc_spectrum
s = calc_spectrum(1900, 2300,         # cm-1
                  isotope='1,2,3',
                  pressure=1.01325,   # bar
                  Tgas=700,           # K
                  mole_fraction={"CO":0.1, "CO2":0.1},
                  path_length=1,      # cm
                  wstep='auto',
                  databank='hitran',  # or use 'hitemp'
                  )

Raises:

  File "d:\github\radis\radis\lbl\calc.py", line 482, in calc_spectrum
    s = MergeSlabs(*s_list)

  File "d:\github\radis\radis\los\slabs.py", line 658, in MergeSlabs
    slabs = resample_slabs(

  File "d:\github\radis\radis\los\slabs.py", line 438, in resample_slabs
    raise ValueError(

ValueError: All wavelengths/wavenumbers must be the same for multi slabs configurations. Consider using `resample='intersect'` or `resample='full'`

🎯 Expected behavior

💡 Possible solutions

@anandxkumar we could simply use "resample='intersect'" in s = MergeSlabs(*s_list) (calc.py, l482). However it may be more efficient to already generate the spectra with the same spectral range (even if the resolution is too high for some of them), to avoid later interpolations. We could :

either evaluate the best wstep for all spectra before computing them, but that requires some refactoring. Let's not.
or, in the loop over all spectra in calc.py, send something like "(wstep_last_spectrum, 'auto')", evaluated as "_take the minimum of wstep_lastspectrum, and the value you'll find with 'auto'". Should be easier to implement, and at least if you happen to need a high resolution spectrum all the subsequent spectra will be computed with the same high resolution.

Can you think of another way?

anandxkumar commented 2 years ago

resample='intersect works fine. But since we have different wstep values for both spectrums I'm not sure what value of wstep when taking the intersection of the parameters. It shows wstep = 'N/A' for the above example. I assume wstep=0.01 is used?

The second solution seems better, but it will involve precomputing all auto values of wstep for each spectrum because we are taking the minimum value so we need to loop over all the spectrums to get the minimum value, and then after getting the minimum value we will use another loop to generate all the spectrums and then merge them. If that's the case, isn't resample='intersect a better option? One other way will be to take the first spectrum wstep value as default, but at the cost of resolution for some higher resolution spectrum?

erwanp commented 2 years ago

with resample='intersect',, you'll get a 'wstep' specifically for each Spectrum ; then they are interpolated eventually ; but the final value doesn't seem to appear in the final spectrum conditions.

My suggestion was a bit different from pre-computing all wstep : i'd rather just use the current minimum. So as you go through the loop wstep can only decrease, or stay constant.

anandxkumar commented 2 years ago

Just a doubt, let us suppose a scenario, for 1st spectrum we have wstep = 0. 5, for second 0.4, and for a third spectrum 0.3 Since we are choosing min(wstep_prev, 'auto'), initially wstep = 0.5 and 1st spectrum gets appended using 0.5, then for the 2nd spectrum, wstep will get updated to 0.4, so for s2 wstep = 0.4, similarly s3 wstep =0.3

now each spectrum has a different wstep value, which will give rise to the same error. And again we will have to use resample='intersect' to resolve it. The advantage of this method will be that there will be fewer interpolations.

erwanp commented 2 years ago

Oh sure, I wasn't clear : we'll use resample='intersect' anyway, and interpolation will fix the problel. But for a large number of spectra (think of a line-of-sight with 100+ spectra) with a randomly distributed minimal wstep, once you've reached the minimum you won't need any further interpolation. And if, as a user, you know how the auto-wstep is done, you may want to start with the spectrum you expect will have the lowest wstep.

anandxkumar commented 2 years ago

Ok, I'm on it, will create a PR soon.

radis / radis