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

Improve radis import speed #309

Merged
merged 3 commits into from Jul 7, 2021
Merged

Conversation

erwanp
Copy link
Member

@erwanp erwanp commented Jul 7, 2021

Description

Improved RADIS import time by delaying the import of packages not necessarily used immediatly (or ever).
(also added a test for Cantera calculations on the default Travis test environment)

Tested with tuna

python -X importtime -c "import radis" 2> radimport.log
tuna radimport.log

Before :

image
image

After:

image

So import is 75% faster (7s -> 4s) !

Delayed :

What's left (not for this PR)

  • Astropy units . ; it's hard to delay Astropy because we need Astropy.units to know if user give Astropy.units. A way would be to "duck-test" it; i.e. evaluate a property.
  • Numba & underlying llvm : is used here and there; in particular for Whiting's JIT. Would be a lot of work to delay this ; and may leave some bugs undected.
  • Pandas / Scipy / Numpy are required anyway.

Notice the time it takes Numba to compute Whiting's JIT.

@codecov-commenter
Copy link

Codecov Report

Merging #309 (a654d6b) into develop (a85f2fe) will decrease coverage by 0.20%.
The diff coverage is 14.70%.

@@             Coverage Diff             @@
##           develop     #309      +/-   ##
===========================================
- Coverage    74.51%   74.31%   -0.21%     
===========================================
  Files          121      122       +1     
  Lines        14806    14823      +17     
===========================================
- Hits         11032    11015      -17     
- Misses        3774     3808      +34     

@erwanp erwanp changed the title (WIP) Improve import speed Improve radis import speed Jul 7, 2021
@erwanp erwanp added performance refactor requires changes in code architecture labels Jul 7, 2021
@erwanp erwanp added this to the 0.9.30 milestone Jul 7, 2021
@erwanp erwanp mentioned this pull request Jul 7, 2021
84 tasks
@encrypted-soul
Copy link
Member

encrypted-soul commented Jul 7, 2021

@erwanp Does the tool render different maps on different machines? I fetched this branch and ran tuna and I seemed to receive a different map? But there was significant improvement with this branch though.

@@ -79,6 +77,9 @@ def fetch_astroquery(
:py:attr:`astroquery.query.BaseQuery.cache_location`

"""
from astropy import units as u
Copy link
Member

Choose a reason for hiding this comment

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

Basically import packages when necessary (?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes ! Imports within functions is harder to maintain so you don't want to do that for things you'll have to load anyway.

For astropy.units it didn't really work; as it's still imported at one point to check the inputs.

@erwanp erwanp merged commit ea7b606 into radis:develop Jul 7, 2021
@erwanp erwanp deleted the improve-import-speed branch July 7, 2021 21:32
@erwanp erwanp mentioned this pull request Aug 4, 2021
erwanp added a commit to erwanp/radis that referenced this pull request Aug 9, 2021
(saves 2s on import. check with code of radis#309)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance refactor requires changes in code architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants