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

Add: SpecDatabase Example in Gallery #315

Merged
merged 5 commits into from Jul 15, 2021
Merged

Conversation

anandxkumar
Copy link
Collaborator

Description

This pull request adds example of SpecDatabase in Gallery.
Screenshot from 2021-07-14 00-41-14

@erwanp
Copy link
Member

erwanp commented Jul 13, 2021

Hello !! Nice to see SpecDatabase being advertised.

A few things to make it great :

  • for the example to work on ReadTheDocs, they'll need to be generated online. For this you cannot use a local database. fetch_hitemp("CO") may work; but it's a slightly big database (no good for 🌳 , remember it's run on every merge to develop!); what about using "OH" for your example ?
  • for the image, can you try to use plot_cond which plots a 2D map of the database conditions ?
  • could you add a 4th method where you make use of the init_database class of SpectrumFactory ?
  • can you add some clickable links to the main classes and methods in the description, i.e. :py:class:`~radis.tools.database.SpecDatabase` ; same with plot_cond and init_database ?
  • last but not least, the example gallery can appear in the docstrings of the functions and methods used. For that, you'll need to add .. minigallery:: radis.PATH/TO/FUNCTION in their docstrings. See Add (more) mini galleries of examples in Documentation #136 to know which path to use

@erwanp erwanp added the documentation related to the docs https://radis.readthedocs.io/ label Jul 13, 2021
@erwanp
Copy link
Member

erwanp commented Jul 13, 2021

@gugzy and @CorentinGrimaldi , adding you for Review as you use the SpecDatabases ; if you think of features / use cases that should appear on the example gallery !

@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2021

Codecov Report

Merging #315 (787bb1c) into develop (3dabb6e) will increase coverage by 0.05%.
The diff coverage is 33.33%.

@@             Coverage Diff             @@
##           develop     #315      +/-   ##
===========================================
+ Coverage    74.30%   74.36%   +0.05%     
===========================================
  Files          122      122              
  Lines        14830    14837       +7     
===========================================
+ Hits         11020    11034      +14     
+ Misses        3810     3803       -7     

@@ -1686,7 +1688,7 @@ def plot_cond(self, cond_x, cond_y, z_value=None, nfig=None):

# Overlay color
if z_value is not None:
assert (len(z_value)) == len(self.df)
assert (len(self.df[z_value])) == len(self.df)

z = np.array(z_value) ** 0.5 # because the lower the better
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@erwanp I believe the above line assert (len(z_value)) == len(self.df) needs to be changed into assert (len(self.df[z_value])) == len(self.df) because z_value is just the column name.

Also the above below gives error:

  z = np.array(z_value) ** 0.5  # because the lower the better

TypeError: ufunc 'power' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe

Copy link
Member

Choose a reason for hiding this comment

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

I looks like that z_value could be used as an external array. What about :

  • if z_value is a str ; and a column name, then use z_value = self.df[z_value]
  • else let it as it is (so the user can still use an external array of any kind)

np.array(z_value) ** 0.5
looks like something added for specific (old) purposes, it could be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Plotted: db_new.plot_cond('Tgas', 'wstep', 'wstep') where wstep value was decreasing, got the below plot-

Figure_1

I believe the color of the point should be different instead of the whole area or was this intended as we are using ax.contourf which colors the axis instead of point?

Copy link
Member

Choose a reason for hiding this comment

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

I think I use it to plot the residuals in Fitroom, if you had a look at it. So the area is what we want. Let's use it : maybe compute small matrix of Tgas and wstep (15-20 spectra max, but larger ranges, like up to 3000 K to show some differences) , and plot the calculation time as the Z_label?

It's just to inspire people, no need to spend much time here :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a better example where I'm halving the wstep at every iteration. Now the difference is clearer and we can infer that Violent -> Minimum calc time, yellow -> Maximum Calc Time. Also, the max time taken is 0.17s and overall takes around 0. 40 seconds to fully execute it so it's not expensive.

Screenshot from 2021-07-15 03-49-33

@anandxkumar
Copy link
Collaborator Author

Screenshot from 2021-07-14 19-18-15-1

Plotting 2 graphs, one simple plot of radiance_noslit of different spectrums in SpecDatabase and other Tgas vs wstep using plot_cond(). Any change in the example?

Also added the mini gallery to SpecDatabase, plot_cond, and init_database. All getting properly rendered.

Copy link
Member

@erwanp erwanp left a comment

Choose a reason for hiding this comment

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

Tiny changes, looks great!!

With the 2 graphs, which one appear in the list of galleries ? It should be the one from plot_cond.
( I don't think we need to plot the spectra anyway. There are already many Gallery examples with spectra)

@@ -1686,7 +1688,7 @@ def plot_cond(self, cond_x, cond_y, z_value=None, nfig=None):

# Overlay color
if z_value is not None:
assert (len(z_value)) == len(self.df)
assert (len(self.df[z_value])) == len(self.df)

z = np.array(z_value) ** 0.5 # because the lower the better
Copy link
Member

Choose a reason for hiding this comment

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

I looks like that z_value could be used as an external array. What about :

  • if z_value is a str ; and a column name, then use z_value = self.df[z_value]
  • else let it as it is (so the user can still use an external array of any kind)

np.array(z_value) ** 0.5
looks like something added for specific (old) purposes, it could be removed.

radis/tools/database.py Show resolved Hide resolved
@erwanp
Copy link
Member

erwanp commented Jul 15, 2021

Merging to see if it works on the online docs!

@erwanp erwanp merged commit c2c2c2f into radis:develop Jul 15, 2021
@erwanp erwanp added this to the 0.9.30 milestone Jul 15, 2021
@erwanp
Copy link
Member

erwanp commented Jul 15, 2021

Hello @anandxkumar ; the docs build failed :

generating gallery for auto_examples... [ 90%] plot_SpecDatabase.py
WARNING: /home/docs/checkouts/readthedocs.org/user_builds/radis/checkouts/develop/examples/plot_SpecDatabase.py failed to execute correctly: Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/radis/checkouts/develop/examples/plot_SpecDatabase.py", line 37, in <module>
    db = SpecDatabase(my_folder, lazy_loading=False)
  File "/home/docs/checkouts/readthedocs.org/user_builds/radis/envs/develop/lib/python3.6/site-packages/radis/tools/database.py", line 1867, in __init__
    os.mkdir(path)
FileNotFoundError: [Errno 2] No such file or directory: '/home/pipebomb/Desktop/SpecDatabase_Test/'

generating gallery for auto_examples... [100%] plot_linestrengths.py

Can you use relative links to build the database ?


Also; can you use the new PR to build a (wstep, Tgas) matrix of spectra rather than 25 spectra with different Tgas. Will be good-looking with plot_cond.
Like wstep = [0.001 :: 0.1, 4] ; Tgas = [300 :: 3000, 5 ] (20 spectra total)
(warnings={"AccuracyError":"ignore"} may be needed

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/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants