-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
Reorganize reaction rate classes #1223
Conversation
@speth ... I have absolutely no problem with this proposition (it makes sense). Would it make sense to also introduce PS: also marked this as relevant for the beta release ... PPS: For #1219 this is just 96d42a0 and bcf6ca7 (and potentially ab47b50) |
Codecov Report
@@ Coverage Diff @@
## main #1223 +/- ##
==========================================
- Coverage 65.44% 65.44% -0.01%
==========================================
Files 320 327 +7
Lines 46321 46321
Branches 19688 19688
==========================================
- Hits 30315 30314 -1
Misses 13475 13475
- Partials 2531 2532 +1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
I like the changes in bcf6ca7. I actually think they make the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I am aware, this is just code reorganization …
Changes proposed in this pull request
ReactionRate
(e.g.TwoTempPlasmaRate
) to a header file of its own, along with the implementation of the correspondingReactionData
class. By making rate implementations self-contained, I think this makes it easier for users to see how to add new ones of their own.ReactionFactory
declaration into an internal header file, so it can be removed after 2.6 without having to go through a deprecation cycle. (this class was introduced after Cantera 2.5, and so has never been a part of the released API)update_C
andupdateRC
methods ofChebyshevRate
andPlogRate
, which are used directly only by the "legacy" rate methods.Checklist
scons build
&scons test
) and unit tests address code coverage