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

Consider config option for numba recompile of montecarlo_main_loop #1908

Open
andrewfullard opened this issue Feb 23, 2022 · 2 comments
Open

Comments

@andrewfullard
Copy link
Contributor

It's possible that the recompile() call added in #1901 will cause performance problems in the future. A config option to disable/enable it would be appropriate.

@Rodot-
Copy link
Contributor

Rodot- commented Feb 24, 2022

#1901 Now no longer include the recompile and we're not sure about possible consequences! Once merged, this should be a priority issue, if not one now

wkerzendorf added a commit to wkerzendorf/tardis that referenced this issue Feb 24, 2022
wkerzendorf added a commit that referenced this issue Feb 24, 2022
* Add calculation of bound-free opacity for numba MC

* Fix bound-free frequency sorting error

* Add determination of macro atom activation level after bound-free absorption

skip-checks: true

* Fix sorting error in bound-free opacity interpolator

* Calculate factor used in free-free cooling from fundamental constants

* Add determination of macro atom activation level after continuum absorption

* Fix renaming bug

* Add calculation of free-free opacity

* Add note about free-free Gaunt factor

* Add function for combined bf and ff opacity calculation

* Make it easier to retrieve the transition_idx of a MA deactivation channel

* Swap level order in transition_idx references to deactivation

* Reformat some lines

* Add transition_types and idxs for MA deactivation by cooling

* Add mask for retrieving transition probabilities of non continuum species

* Combine the Markov chain and normal transition probabilities

* Reformat some lines

* Replace dummy k-packet idx in interaction handling

* Change output name of combined continuum transition probabilities

* Add block references for combined continuum transition probabilities

* Activate macro atom by level idx instead of line idx

* Make macro atom data a plasma property to enable modularity

* Split transition probabilities and transition info

* Fix macro atom test

* Adapt transition types in plasma to match the ones in the numba macro atom

* Make the macro atom return the transition type

* Add individual fb cooling probabilities to MC transition probabilities

* Make bound-free cross sections available in the MC calculation

* Initialize the arrays for the continuum estimators

* Convert continuum estimator arrays to DataFrames

* Make t_electrons available in MC calculation

* Propagate continuum estimators into plasma

* Add continuum estimators to Numba estimators

* Reformat some lines

* Made path selection a little bit faster and simplifiedw

* Add sorted list of threshold frequencies for photoionization to Numba MC

* Added selection fucntion for continuum process, WIP

* Moved continuum selection inside of tracepacket, still need a docstring

* added plasma properties that perform the sampling of the free-free and free-bound emission processes

* restrcited sampling range between continuum ids

* Record bound-free estimators during Numba MC

* Began work on Continuum jitclass

* Implemented the Continuum as a jitclass which caches the state at a given shell_id and fequency

* Added the continuum samplers to the  attribute of the continuum processes file.  Fixed implementation after merge

* Added temporary change to make all continuum processes thomson scatter so that the simulation finished in a finite amount of time for testing during development

* fixed the continuum samplers so they are now properly processed.  Also fixed some bugs in the samplers

* Reworked the continuum process selection function a little bit to work with the new continuum jitclass

* started addition of free-free emission handler

* Add calculation of bound-free opacity for numba MC

* Fix bound-free frequency sorting error

* Add determination of macro atom activation level after bound-free absorption

skip-checks: true

* Fix sorting error in bound-free opacity interpolator

* Calculate factor used in free-free cooling from fundamental constants

* Add determination of macro atom activation level after continuum absorption

* Fix renaming bug

* Add calculation of free-free opacity

* Add note about free-free Gaunt factor

* Add function for combined bf and ff opacity calculation

* Make it easier to retrieve the transition_idx of a MA deactivation channel

* Swap level order in transition_idx references to deactivation

* Reformat some lines

* Add transition_types and idxs for MA deactivation by cooling

* Add mask for retrieving transition probabilities of non continuum species

* Combine the Markov chain and normal transition probabilities

* Reformat some lines

* Replace dummy k-packet idx in interaction handling

* Change output name of combined continuum transition probabilities

* Add block references for combined continuum transition probabilities

* Activate macro atom by level idx instead of line idx

* Make macro atom data a plasma property to enable modularity

* Split transition probabilities and transition info

* Fix macro atom test

* Adapt transition types in plasma to match the ones in the numba macro atom

* Make the macro atom return the transition type

* Add individual fb cooling probabilities to MC transition probabilities

* Make bound-free cross sections available in the MC calculation

* Initialize the arrays for the continuum estimators

* Convert continuum estimator arrays to DataFrames

* Make t_electrons available in MC calculation

* Propagate continuum estimators into plasma

* Add continuum estimators to Numba estimators

* Reformat some lines

* Add sorted list of threshold frequencies for photoionization to Numba MC

* Record bound-free estimators during Numba MC

* Add estimator for stimulated recombination cooling rate

* Add column name to estimator DataFrames

* updated interactions to work with new macroatom

* Added continuum process frquency samplers to the continuum jitclass constructor

* Fixed typo from merge conflict

* Fixed spacing

* small formatting updates to get the changes to track

* propagated continuum through single packet loop

* Cleaned up file, bound-free and free-free emission pathways should be in the correct place now

* added fixture for a new continuum object

* Added placeholder test for calculating the continuum opacities.  I'm weary to put in exact values until we're done

* reworked the contruction of the continuum class so that only a plasma object needs to be passed

* Updated interaction to now get the macro activation level from the continuum object

* refer to previous commit

* Added the continuum_process into the single packet loop so packets can now actually interact with the contniuum

* various small bugfixes to get the numba funcs to compile properly

* Removed extra functions

* Merge pull request #5 from chvogl/monte-carlo-continuum (#4)

* Reworked the continuum process selection function a little bit to work with the new continuum jitclass

* started addition of free-free emission handler

* updated interactions to work with new macroatom

* Added continuum process frquency samplers to the continuum jitclass constructor

* Fixed typo from merge conflict

* Fixed spacing

* small formatting updates to get the changes to track

* propagated continuum through single packet loop

* Cleaned up file, bound-free and free-free emission pathways should be in the correct place now

* added fixture for a new continuum object

* Added placeholder test for calculating the continuum opacities.  I'm weary to put in exact values until we're done

* reworked the contruction of the continuum class so that only a plasma object needs to be passed

* Updated interaction to now get the macro activation level from the continuum object

* refer to previous commit

* Added the continuum_process into the single packet loop so packets can now actually interact with the contniuum

* various small bugfixes to get the numba funcs to compile properly

* Removed extra functions

* Fix bug in calculation of corrected photoionization rate coefficient

* Format some lines

* Update tardis/montecarlo/montecarlo_numba/interaction.py

Co-authored-by: Christian Vogl <[email protected]>

* No longer recalculate the local continuum opacities before interacting

* Resolved comment from Christian, do the same process for BF cooling as BF emission

* Minor formatting, currently we still don't actually run through any continuum processes because the new macroatom is not in

* Resolved a couple of comments. (#5)

* Reworked the continuum process selection function a little bit to work with the new continuum jitclass

* started addition of free-free emission handler

* updated interactions to work with new macroatom

* Added continuum process frquency samplers to the continuum jitclass constructor

* Fixed typo from merge conflict

* Fixed spacing

* small formatting updates to get the changes to track

* propagated continuum through single packet loop

* Cleaned up file, bound-free and free-free emission pathways should be in the correct place now

* added fixture for a new continuum object

* Added placeholder test for calculating the continuum opacities.  I'm weary to put in exact values until we're done

* reworked the contruction of the continuum class so that only a plasma object needs to be passed

* Updated interaction to now get the macro activation level from the continuum object

* refer to previous commit

* Added the continuum_process into the single packet loop so packets can now actually interact with the contniuum

* various small bugfixes to get the numba funcs to compile properly

* Removed extra functions

* No longer recalculate the local continuum opacities before interacting

* Resolved comment from Christian, do the same process for BF cooling as BF emission

* Minor formatting, currently we still don't actually run through any continuum processes because the new macroatom is not in

* Switch to macro atom data and probabilities with continuum processes

* Raise an exception if more than one two-photon decays are requested

* Add transition info for two-photon decays to numba macro atom data

* Add frequency sampler for two-photon emission

* Updated the bound-free frequency sampler.  Fixed an issue arising from the indexing

* Now allows for continuum processes to occur

* Added proper interaction type to r_packet for continuum processes

* Added bound-free cooling and adiabatic cooling, debugging in progress.  Current issue with macroatom running out of the block

* fixed updatin gof distance_continuum in r_packet, set the chi_bf to zero when there's no cuirrent continuum

* removed print statements for packet index

* Removed excess print statements, added some comments for clarity

* removed a couple commented-out print statements

* added docstrings

* fixed error from typo

* Added conditional compilation of the Continuum jitclass such that normal methods are replaces with dummy methods when we are not worrying about the continuum opacities.  Updated the conditional initialization of the plasma as well accordingly

* updated docstring for montecarlo main loop, should be noted that passing a CPUDispatcher used as a constructor to the main loop function is slow according to numba docs, in the future a factory function should generate the main loop

* added some fixed for running without continuum species

* Added proper flag for adiabatic cooling

* removed extra print statement to check for continuum processes in testing

* removed commented out print statements

* Added configuration parameter for inclusion of continuum processes and had it set during the initialization of the simulation object.  Will be useful for future numba optimization as a compile-time constant

* switched check for continuum processes to rely on config parameter

* Added conditional paths for continuum interactions in the macroatom events based upon compile time constant

* Fixed vpackets throwing a close line error

* Implimented partial fixed for formal integral, development ongoing

* Fixed merge issues

* Fix treatment at end of line list

* Updated continuum interactions to appropriately use comoving frequencies when exciting a macroatom

* added copy method to continuum class

* Added continuum interactions to vpacket

* added continuum interactions to vpacket

* Fixed arg ordering so args come before kwargs, removed conversion of lists to numpy arrays before concatenations

* Vpackets now all share one continuum object since they go sequentially

* decided to go with copies of continuum

* Defer updating continuum if parameters have not changed since last call

* removed check for updating continuum since vpackets always cross a shell boundary before updating

* went back to single instance of continuum to share among vpackets since deffering updating of the continuum is unneccesary

* realtivity check

* Added a step to recompile the main_loop to make sure things get properly set on new runs

* Removed reindexing of photoionization data to be compatable with carsus

* Added fix from chvogl for handling duplicate lines

* Removed the recompile since it caused issues with the lifetimes of some objects

* Fixed bug introduced by relativity fix.  Now recompiling the main loops works and the relativity flag has a namespace

* fixed problems
Co-authored-by: Andreas Flörs <[email protected]>
Co-authored-by: Christian Vogl <[email protected]>

* fixed tracker

* Added

Co-authored-by: Jack O'Brien <[email protected]>

* trying to fix the tests

Co-authored-by: Jack O'Brien <[email protected]>

* remove recompile step due to some numba screwup related #1908

Co-authored-by Jack O'Brien <[email protected]>

Co-authored-by: Christian Vogl <[email protected]>
Co-authored-by: rodot- <[email protected]>
Co-authored-by: Jack O'Brien <[email protected]>
Co-authored-by: Sona Chitchyan <[email protected]>
@andrewfullard
Copy link
Contributor Author

This is likely resolved by the change in switches to be outside the numba code, e.g. #2719

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants