Skip to content
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

Updated convolve_with_slit with warnings if instrumental function has large near-zero wings #283

Merged
merged 8 commits into from Nov 14, 2021

Conversation

gh4ag
Copy link
Contributor

@gh4ag gh4ag commented Jun 9, 2021

updated warnings on convolve_with slit and the documentaiton of the function, modified doc of spectrum to point to the doc of convolve_with_wlit

Description

PR in order to update the documentation and the errors of radis.tools.slit.py convolve_with_slit.

Convolution intervene in mainly 2 places:

  • convolution with physical broadening of spectral features during simulation
  • convolution with instrumental function to fit simulation to experimental spectra

I did my best to explain to the user what to expect out of the convolution product in the case of intrumental functions.

It is important to tell users how convolution could affect data outside of the know spectral range.

I modified apply_slit to point to the Notes I wrote in convolve_with_slit.

I transformed the AssertError of convolve with slit with several warnings, that look both at the spectral range and the FWHM of the input experimental slit. IMO, it will better guide users to what they should do in order to improve the output of the convolution.

radis/tools/slit.py Outdated Show resolved Hide resolved
radis/tools/slit.py Outdated Show resolved Hide resolved
radis/tools/slit.py Show resolved Hide resolved
radis/tools/slit.py Outdated Show resolved Hide resolved
@erwanp erwanp added post-process Related to the post-processing (ex: methods of Spectrum objects) documentation related to the docs https://radis.readthedocs.io/ labels Jun 9, 2021
@erwanp erwanp changed the title Updated convolve_with_wlit Updated convolve_with_slit with warnings if instrumental function has large near-zero wings Jun 10, 2021
@@ -714,15 +787,27 @@ def get_FWHM(w, I, return_index=False):
"""
# TODO: Linearly interpolate at the boundary? insignificant for large number of points

upper = np.argwhere(I >= I.max() / 2)
half = I.max() / 2
I_offset = I - half
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand why it's needed : the slit is zero on the sides.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This steps makes interpolation easier conceptually to find the FWHM by solving the question "when do my graph crosses the y=0 axis". The function get_FWHM assumes the slit to have only 1 peak and to go to zero monotously away from the peak. You can modify it if you want, to find the y=half - but then you have to change the helper function get_zero as well (should be easy).

I don't think I understood your comment. This function doesn't look at the sides of the slit here. It looks at the FWHM, so close to the center of the slit.

Copy link
Contributor Author

@gh4ag gh4ag Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just implemented the TODO XD

TODO: Linearly interpolate at the boundary? insignificant for large number of points

@codecov-commenter
Copy link

Codecov Report

Merging #283 (7ef1d48) into develop (658e629) will increase coverage by 0.00%.
The diff coverage is 77.77%.

@@           Coverage Diff            @@
##           develop     #283   +/-   ##
========================================
  Coverage    75.24%   75.25%           
========================================
  Files          119      119           
  Lines        14273    14284   +11     
========================================
+ Hits         10740    10749    +9     
- Misses        3533     3535    +2     

@erwanp erwanp added this to the 0.9.30 milestone Jun 12, 2021
@erwanp erwanp modified the milestones: 0.9.30, 0.9.31 Aug 10, 2021
@erwanp erwanp modified the milestones: 0.10.3, 0.10.4 Sep 23, 2021
@erwanp erwanp modified the milestones: 0.10.4, 0.10.5 Nov 1, 2021
@erwanp erwanp modified the milestones: 0.12, 0.11.0 Nov 14, 2021
@erwanp erwanp merged commit c4a3d74 into radis:develop Nov 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation related to the docs https://radis.readthedocs.io/ post-process Related to the post-processing (ex: methods of Spectrum objects)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants