radis / radis

🌱 A fast line-by-line code for high-resolution infrared molecular spectra
http://radis.github.io/
GNU Lesser General Public License v3.0
214 stars 126 forks source link

check radiance calculation in GPU interactive mode #461

Open erwanp opened 2 years ago

erwanp commented 2 years ago

https://github.com/radis/radis/blob/a0c9bf3d9444b05ad4196959398eddf1879f8d40/radis/lbl/factory.py#L1363

As i read these lines, the parameter radiance is recomputed from emissivity although it should be radiance_noslit (and slit should be applied afterwards)

Same goes for the calculation of emissivity above : emissivity = 1 - transmittance ; this is correct if transmittance is the non-convolved version.

erwanp commented 2 years ago

@dcmvdbekerom let's check this sometime !

dcmvdbekerom commented 2 years ago

I noticed in the example plot_gpu_widgets.py if you tweak the sliders only slightly, the spectrum shoots up. This means the update calculates the spectrum differently (wrong) than the initial spectrum.

This could be related to the bug you mentioned, I will have a look this weekend.

dcmvdbekerom commented 2 years ago

We could solve this pretty easily, but I would like to propose a more rigorous solution;

Currently we calculate abscoeff, absorbance, emissivity, emissivity_noslit, transmittance_noslit, radiance_noslit, and transmittance outside of the Spectrum object, and then instantiate the object with all these as arguments through the quantities dict.

Because these are all related however, I would suggest that the Spectrum object should only be instantiated with an abscoeff argument, and calculates all the derived quantities during instantiation. It should do so with a method Spectrum.update_quantities(abscoeff), such that it could later be updated as is needed in the sf.eq_spectrum_gpu_interactive() function. This guarantees that all the quantities within a spectrum object will always be linked to eachother.

Would like to hear your thoughts @erwanp!

erwanp commented 2 years ago

Agreed, and already implemented.

Spectrum.update("radiance_noslit")

It can do so from abscoeff, and will require emisscoeff or an equivalent spectral array under nnoequilibrium.

And actually, starting from a recent Radis version, do don't even need update() anymore. Just use Spectrum.get() or Spectrum.plot() with whatever quantity and it will compute it if possible.

erwanp commented 2 years ago

@dcmvdbekerom is this fixed by #514 ?

dcmvdbekerom commented 2 years ago

I'm working on it but don't know if I can get it fixed today..

dcmvdbekerom commented 1 year ago

@erwanp i don't remember.. was this issue solved?

erwanp commented 1 year ago

We didn't check, so no

dcmvdbekerom commented 1 year ago

Keeping this thread alive..