Numba formal integral #1549
Merged
Numba formal integral #1549
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…inal C version for consistency
…is a numba implementation
…mba compatable types
…updated the njit dict to no longer use fastmath
…he interpolated shells
|
/AzurePipelines run compare-refdata |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
One thing to note: I removed tardis/montecarlo/tests/test_montecarlo.py since it was mostly C tests, but when I merged I noticed there had been some updates to it. Should this file be added back in and modified to only do the numba side of things or is it fine being removed? @andrewfullard |
…at least set up here. Also, readded the setup_package.py file
|
Okay, well, I added them back in just in case, they all skip anyway and I modified them not to include any C code references. But I left them just in case we want to move some of these tests over to a different file later so the work isn't lost. I think this is ready to go |
wkerzendorf
reviewed
Apr 30, 2021
| @@ -1,53 +1,236 @@ | |||
|
|
|||
| import sys | |||
|
|
|||
wkerzendorf
reviewed
Apr 30, 2021
Comment on lines
16
to
17
| from tardis.montecarlo.montecarlo_numba.numba_interface \ | ||
| import numba_plasma_initialize, NumbaModel, NumbaPlasma |
There was a problem hiding this comment.
Suggested change
| from tardis.montecarlo.montecarlo_numba.numba_interface \ | |
| import numba_plasma_initialize, NumbaModel, NumbaPlasma | |
| from tardis.montecarlo.montecarlo_numba.numba_interface ( | |
| import numba_plasma_initialize, NumbaModel, NumbaPlasma) |
wkerzendorf
reviewed
Apr 30, 2021
| from tardis.montecarlo.spectrum import TARDISSpectrum | ||
|
|
||
| C_INV = 3.33564e-11 | ||
| M_PI = np.arccos(-1) | ||
| KB_CGS = 1.3806488e-16 | ||
| H_CGS = 6.62606957e-27 | ||
| SIGMA_THOMSON = 6.652486e-25 | ||
|
|
||
| #SIGMA_THOMSON = 6.652486e-25 |
There was a problem hiding this comment.
Suggested change
| #SIGMA_THOMSON = 6.652486e-25 |
wkerzendorf
reviewed
Apr 30, 2021
| Helper class for performing the formal integral | ||
| with numba. | ||
| ''' | ||
| #def __init__(self, model, plasma, runner, points=1000): |
There was a problem hiding this comment.
Suggested change
| #def __init__(self, model, plasma, runner, points=1000): |
wkerzendorf
reviewed
Apr 30, 2021
Comment on lines
669
to
671
| #for i in range(N): | ||
| # opp[i] = R_max / (N - 1) * (i) | ||
| #return opp |
There was a problem hiding this comment.
Suggested change
| #for i in range(N): | |
| # opp[i] = R_max / (N - 1) * (i) | |
| #return opp |
wkerzendorf
approved these changes
Apr 30, 2021
2 tasks
atharva-2001
pushed a commit
to atharva-2001/tardis
that referenced
this pull request
Oct 1, 2021
* Readded the numba formal integral changes * Update base.py to match main * Reverted formal integral instantiation to be more similar to the original C version for consistency * Moved numba implimentation to the montecarlo_numba folder since this is a numba implementation * Removed extra ravel * Added back old formal integral for testing * Added test for function in the numba formal integral * Changed the way the shell size is computed, should be correct now * nevermind * Moved numba formal integral tests so that paths work out for testing framework * Moved pointer incrementation to the end of the loop * Fixed typo * Switched tests to use numba models * numba models instantiate with proper data types now * Fixed numba_formal_integral tests to instantiate numba models with numba compatable types * Added a couple docstrings with some notes about future improvements * Streamlined the interface slightly to better catch bugs * Added some debug statements to watch array sizes in tests * Changed way size_line and size_shell are created using tau_sobolevs * Changed the most inner for-loop to be more similar to the C code and updated the njit dict to no longer use fastmath * Added back in the check for interpolate shells * Added back in old code for interpolate shells and now computes over the interpolated shells * Fixed interpolate shells function and made sure dataframes are properly returned when computing the intergral * Fixed bug in counter for Jred_lu pointer * More explicit about typing. the value for SIGMA_THOMSON now comes from the numba config (which is should have in the past). Should now match C code within test criteria * Finished tests for the numba formal integral helper functions * Added some docstrings and removed some commented out code * Removed old import of C formal integral * Removed deprecated formal integral import * Changed test to generate a new numba model as to not interfere with other tests * removed old C interface * Updated tests and environment to exclude compilers and Cython * Reformatted conftest so black doesn't complain * Removed commented out lines, made cleaner * Added back montecarlo tests since it apepars some r_packet tests are at least set up here. Also, readded the setup_package.py file * Removed commented out packages from the environment file * Slight optimization and readability improvements * Further optimization, now pre-computes indices of next resonance point * Removed old binary search, made formatting fixed Co-authored-by: Andrew Fullard <andrewgfullard@gmail.com> Co-authored-by: Wolfgang Kerzendorf <wkerzendorf@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Implementation of the formal integral with numba. New tests added for the numba version of various helper functions used to compute the formal integral.
Motivation and context
We would like to move TARDIS fully away from C. The formal integral was the last component of TARDIS written entirely in C.
How has this been tested?
New tests have been added for the numba implementation. Full runs that compute formal integrals are compared to reference-data generated using the C formal integral. All tests pass.
Type of change
Refactor from C to Python
Checklist