Conditional rate updates for MultiRate reactions#1151
Conversation
Updating the "shared data" for a rate type returns flags indicating whether the "update" or "eval" methods for each reaction of that type need to be called to get new values of the rate constant.
Codecov Report
@@ Coverage Diff @@
## main #1151 +/- ##
==========================================
+ Coverage 73.73% 73.76% +0.03%
==========================================
Files 370 370
Lines 48927 48983 +56
==========================================
+ Hits 36077 36133 +56
Misses 12850 12850
Continue to review full report at Codecov.
|
ischoegl
left a comment
ischoegl
left a comment
There was a problem hiding this comment.
@speth … this looks like a really good solution to the update process. Thanks for looking into this, as this was one of the items that needed to be taken care of. Hadn’t thought of moving the update checks into ReactionData before seeing this, but I like this solution quite a bit, as it appears to resolve multiple issues once and for all.
There was a problem hiding this comment.
Now that ReactionData has become a lot smarter, it may make sense to have a pure abstract parent class/struct for documentation purposes, where each of the current classes is a specialization. I don't want to include this in this PR, but I think it would make sense to implement this prior to 2.6.
2 participants
Changes proposed in this pull request
RateData::updatemethod (executed once per reaction type) to determine whether or not bothupdateFromStructandevalFromStructneed to be called for all reactions of that type.BulkKinetics::m_bulk_ratesto be skipped incorrectlyIf applicable, provide an example illustrating new features this pull request is introducing
The timing results from
custom_reactions.pyshow an improvement compared to the "legacy" framework, with the new framework now outperforming the "legacy" one.Current
mainbranch:This PR:
Checklist
scons build&scons test) and unit tests address code coverage