Add predict_time(), make broadening_method = "voigt" as default , add default truncation of 50 cm-1 #343

anandxkumar · 2021-08-12T18:47:18Z

Description

This pull request adds the predict_time() method in SpectrumFactory to precompute the estimated time taken by Legacy>Voigt, LDM>Voigt, and LDM>FFT methods.

Splits broadening_max_width into 2 parameters - neighbour_lines and truncation.

Sets DIT>Voigt as default method with a truncation=300 cm-1 and neighbour_lines = 0.
For LBL>Voigt, truncation=Spectral_range with neighbour_lines = None

Fixes #340

todo :

user error message if broadening_max_width used; suggestion with correct truncation value

erwanp · 2021-08-13T02:58:19Z

radis/lbl/calc.py

+        broadening_max_width = 300
+        if (wavenum_max - wavenum_min) < 300:
+            broadening_method = "fft"
+        else:
+            broadening_method = "voigt"


Eventually, as discussed in #340 ; we'll just use voigt all the time.
Can you

change broadening_max_width to truncation=Default("300") ; neighbour_lines=0

keep broadening_method everywhere, it's a good addition and can be used to easily switch/commpare

@erwanp in case if the user gives all values that are broadening_max_width and truncation should we use the truncation value and neighbour_lines?
Also what will be the value of neighbour_lines based on different spectral range, optimization and broadening_method or we keep it manual and set it to 0 as default for every case?

radis/lbl/factory.py

radis/test/test_examples.py

radis/lbl/factory.py

…default

…and calc_lineshape_DLM

codecov-commenter · 2021-08-14T21:12:23Z

Codecov Report

Merging #343 (de1f5e5) into develop (71a7478) will increase coverage by 0.50%.
The diff coverage is 86.69%.

@@             Coverage Diff             @@
##           develop     #343      +/-   ##
===========================================
+ Coverage    75.50%   76.01%   +0.50%     
===========================================
  Files          152      152              
  Lines        16769    16954     +185     
===========================================
+ Hits         12662    12888     +226     
+ Misses        4107     4066      -41

erwanp · 2021-08-16T20:05:50Z

Is the calculation mode set to DIT/Voigt by default?

anandxkumar · 2021-08-16T20:08:43Z

Yes with a broadening_max_width =300 and a broadening_max_width = 10 for LBL.

erwanp · 2021-08-16T20:29:22Z

examples/plot_exomol_spectrum.py

@@ -19,6 +19,7 @@
    Tgas=1000,  # K
    mole_fraction=0.1,
    path_length=1,  # cm
+    broadening_method="fft",  # @ dev: Doesn't work with 'voigt'


Do you know what fails?

AttributeError: 'DataFrame' object has no attribute 'selbrd'
Basically voigt uses df.selbrd to calculate the voigt_broadening_HWHM. Seems like the exomol data doesn't have this particular column.

@minouHub you'll be interested

Normally, the other broadening (i.e. airbrd in HITRAN, may be different in ExoMol) is used when selbrd is missing.

erwanp · 2021-08-19T23:08:54Z

radis/lbl/factory.py

+            if optimization is None:
+                if self.verbose >= 3:
+                    printg(
+                        "LDM algorithm not used. Defaulting truncation from {0} to 10".format(


I think we can actually keep 0 in any case.
Well add an a posteriori Accuracy warning if needed

…date wavenumber_calc

erwanp · 2021-08-20T21:15:21Z

radis/lbl/factory.py

@@ -529,38 +530,29 @@ def __init__(
            if optimization is None:
                if self.verbose >= 3:
                    printg(
-                        "LDM algorithm not used. Defaulting truncation from {0} to 10".format(
+                        "LDM algorithm not used. Defaulting truncation from {0} to 0".format(


I made an error here. Default truncation should be 300 cm-1. And default neighbor likes should be 0. Algorithm shouldn't matter (DIT /Voigt and LBL/Voigt have same truncation, only DIT/FFT should raise an error if truncation as its not implemented yet)

DIT/Voigt with 300 cm-1 truncation will be feasible, but I'm not sure about LBL/Voigt. As seen from the benchmarks before, LBL is independent of spectral range i.e. neighbour_lines, but is dependent on the truncation. And for a truncation=300cm-1, the calculation time and memory will be very expensive. So a default 300 cm-1 may not be feasible.

Good point for the performance drop. But at least the physics remain the same, so it's a good reason to have 300 cm-1 always (similar to the remark of @dcmvdbekerom in #340 : we don't want to change the physical output without the user knowing / deciding)

Given DIT/Voigt is the default, it should be fine in most cases.

And anyway, if the predict_time() is given before the broadening step, the user will know.
We can add an extra PerformanceWarning (with suggestion to switch to DIT/Voigt) if predicted calculation time exceeds a few minutes.

…date wavenumber_calc

also minor refactor of register() function in HITEMPDatabaseMAnaager add note that all registred files will only be downloaded when required (as pointed out in radis#306)

works if truncation=None fix truncation being half-with and not lineshape full width (in docstrings and code)

with new offset, no need to distinguish between lines in the range and lines outside the range whose half-lineshape only needs to be summed. (performance ~ same on CO 2100-2200 cm-1 )

fix a bug when database contained 1 isotope in the spectral range but 2+ isotopes were asked

…ing_max_width (full width) fix truncation not appearing in Spectrum conditions if None

…idth' improve an error message

fix a warning

… benchmark

(refactor) do not store full woutrange array, just store a tuple add arange_len function, add tests

… but give a suggestion on what to replace it with

lgtm-com · 2021-08-22T11:16:34Z

This pull request introduces 1 alert when merging de1f5e5 into 1241f08 - view on LGTM.com

new alerts:

1 for Module is imported with 'import' and 'import from'

… benchmark

lgtm-com · 2021-08-23T07:32:45Z

This pull request introduces 1 alert when merging 00720e0 into 1241f08 - view on LGTM.com

new alerts:

1 for Module is imported with 'import' and 'import from'

anandxkumar added 3 commits August 7, 2021 01:50

Adding wL and wG

57c2b62

Add predict_time() in spectrum factory

5f56dce

add: auto method for optimization in calc_spectrum

8ab984a

anandxkumar added this to In progress in GSOC21 : Automatic Lineshape Engine via automation Aug 12, 2021

anandxkumar added interface not related to the physics of the code performance labels Aug 12, 2021

erwanp mentioned this pull request Aug 13, 2021

Add a 50cm-1 lineshape truncation by default #340

Closed

erwanp requested changes Aug 13, 2021

View reviewed changes

erwanp reviewed Aug 13, 2021

View reviewed changes

radis/lbl/factory.py Outdated Show resolved Hide resolved

anandxkumar added 2 commits August 14, 2021 22:12

Switched to broadening_method = voigt and broadening max width = 300 …

da0bc80

…default

Updated predict_time() and printing estimated time in calc_lineshape …

162108e

…and calc_lineshape_DLM

anandxkumar changed the title ~~Add predict_time() and optimization='auto' in calc_spectrum~~ Add predict_time() and make broadening_method = "voigt" as default Aug 14, 2021

Fixed Overpopulation Issue and positioning of Estimated calculation

93efaf0

anandxkumar marked this pull request as ready for review August 16, 2021 19:43

anandxkumar added 2 commits August 19, 2021 14:22

Added arguement truncation and neighbour_lines

e788b42

Removed Broadening_max_width

96292d8

erwanp assigned anandxkumar Aug 19, 2021

anandxkumar added 3 commits August 20, 2021 00:59

Updated truncation and neighbour_lines, and tests

00abcde

Fixed plot_fitting_method conflict

4f815da

Minor test fix

3bd86a8

anandxkumar requested a review from erwanp August 20, 2021 06:54

erwanp reviewed Aug 20, 2021

View reviewed changes

Fix: Merge conflict, select min of truncation, neighbour_lines and up…

6feef61

…date wavenumber_calc

erwanp reviewed Aug 20, 2021

View reviewed changes

erwanp added this to the 0.9.30 milestone Aug 21, 2021

erwanp changed the title ~~Add predict_time() and make broadening_method = "voigt" as default~~ Add predict_time(), make broadening_method = "voigt" as default , add default truncation of 300 cm-1 Aug 21, 2021

Fix: Merge conflict, select min of truncation, neighbour_lines and up…

fcc19dd

…date wavenumber_calc

erwanp added 10 commits August 21, 2021 20:22

(wip)

16a1d5e

fix problem with locale download_date that wasn't parsed properly

cadf223

also minor refactor of register() function in HITEMPDatabaseMAnaager add note that all registred files will only be downloaded when required (as pointed out in radis#306)

use local database instead of downloaded one

b3eba24

fix broadening implementation with truncation

dbb7631

works if truncation=None fix truncation being half-with and not lineshape full width (in docstrings and code)

simplify code that sums all lineshapes :

f993939

with new offset, no need to distinguish between lines in the range and lines outside the range whose half-lineshape only needs to be summed. (performance ~ same on CO 2100-2200 cm-1 )

fix tests

56358e2

fix tests

a29d53d

fix a bug when database contained 1 isotope in the spectral range but 2+ isotopes were asked

fix old test values to match Truncatino (half-width) with old Broaden…

b3ce45b

…ing_max_width (full width) fix truncation not appearing in Spectrum conditions if None

fix warning when highres & lowres spectra are inverted

f87c878

fix SpecDatabase.get(**conditions) when conditions contain None

4c3afe0

erwanp force-pushed the benchmark branch from e6e218f to 4c3afe0 Compare August 21, 2021 18:54

erwanp and others added 8 commits August 21, 2021 21:11

linting

8cfadc6

backward compatibility if loading old Spectrum with 'broadening_max_w…

6cdde03

…idth' improve an error message

Merge benchmark, fixed linting

024fe1d

add truncation tests

78ecbf7

fix a warning

Merge branch 'benchmark' of https://github.com/anandxkumar/radis into…

e3d6adc

… benchmark

fix index error with wstep float division, add test

ec6c6d7

(refactor) do not store full woutrange array, just store a tuple add arange_len function, add tests

backward compatibility : we raise an error if broadening_max_width,…

36fd644

… but give a suggestion on what to replace it with

fix tests

de1f5e5

Merge branch 'benchmark' of https://github.com/anandxkumar/radis into…

cde09af

… benchmark

anandxkumar changed the title ~~Add predict_time(), make broadening_method = "voigt" as default , add default truncation of 300 cm-1~~ Add predict_time(), make broadening_method = "voigt" as default , add default truncation of 50 cm-1 Aug 23, 2021

Set Default truncation to 50cm-1

00720e0

anandxkumar merged commit 710cd38 into radis:develop Aug 23, 2021

GSOC21 : Automatic Lineshape Engine automation moved this from In progress to Done Aug 23, 2021

erwanp mentioned this pull request Aug 25, 2021

0.10.0 (formerly 0.9.30) #273

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add predict_time(), make broadening_method = "voigt" as default , add default truncation of 50 cm-1 #343

Add predict_time(), make broadening_method = "voigt" as default , add default truncation of 50 cm-1 #343

anandxkumar commented Aug 12, 2021 •

edited by erwanp

erwanp Aug 13, 2021

anandxkumar Aug 14, 2021 •

edited

codecov-commenter commented Aug 14, 2021 •

edited

erwanp commented Aug 16, 2021

anandxkumar commented Aug 16, 2021

erwanp Aug 16, 2021

anandxkumar Aug 20, 2021

erwanp Aug 21, 2021

erwanp Aug 19, 2021

erwanp Aug 20, 2021

anandxkumar Aug 21, 2021 •

edited

erwanp Aug 21, 2021 •

edited

lgtm-com bot commented Aug 22, 2021

lgtm-com bot commented Aug 23, 2021

Add predict_time(), make broadening_method = "voigt" as default , add default truncation of 50 cm-1 #343

Add predict_time(), make broadening_method = "voigt" as default , add default truncation of 50 cm-1 #343

Conversation

anandxkumar commented Aug 12, 2021 • edited by erwanp

Description

erwanp Aug 13, 2021

Choose a reason for hiding this comment

anandxkumar Aug 14, 2021 • edited

Choose a reason for hiding this comment

codecov-commenter commented Aug 14, 2021 • edited

Codecov Report

erwanp commented Aug 16, 2021

anandxkumar commented Aug 16, 2021

erwanp Aug 16, 2021

Choose a reason for hiding this comment

anandxkumar Aug 20, 2021

Choose a reason for hiding this comment

erwanp Aug 21, 2021

Choose a reason for hiding this comment

erwanp Aug 19, 2021

Choose a reason for hiding this comment

erwanp Aug 20, 2021

Choose a reason for hiding this comment

anandxkumar Aug 21, 2021 • edited

Choose a reason for hiding this comment

erwanp Aug 21, 2021 • edited

Choose a reason for hiding this comment

lgtm-com bot commented Aug 22, 2021

lgtm-com bot commented Aug 23, 2021

anandxkumar commented Aug 12, 2021 •

edited by erwanp

anandxkumar Aug 14, 2021 •

edited

codecov-commenter commented Aug 14, 2021 •

edited

anandxkumar Aug 21, 2021 •

edited

erwanp Aug 21, 2021 •

edited