Fix reactor mass flow rate #886
Merged
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
Since the mass flow rate can depend on both a function of time and the state of the connected reactors, the only time for which the mass flow rate can be reliably returned is the current time of the reactor network.
Eliminate unnecessary variables m_mdot_in and m_mdot_out, and remove the need to update the mass flow rates in both updateState and evalEqs by using the current (trial) time when updating them in updateState.
Codecov Report
@@ Coverage Diff @@
## master #886 +/- ##
==========================================
- Coverage 70.49% 70.42% -0.08%
==========================================
Files 375 375
Lines 45649 45652 +3
==========================================
- Hits 32182 32152 -30
- Misses 13467 13500 +33
Continue to review full report at Codecov.
|
bryanwweber
approved these changes
Jul 1, 2020
There was a problem hiding this comment.
This looks good to me. One small point for discussion is whether this would be a good time to change the references to the master flow devices, or whether that would be better in another PR. I'm fine either way.
@paulblum these changes will affect what you're working on.
|
I was going to save that for another PR. I'm trying to keep my PRs to a manageable size. 😂 |
|
The foreberance is appreciated 🤣 |
speth
added a commit
to speth/cantera
that referenced
this pull request
Jul 1, 2020
Changes to the blessed output file are a result of updates to atomic weights, and to fixes for how flow devices are updated (Cantera#886)
4 tasks
bryanwweber
pushed a commit
that referenced
this pull request
Jul 3, 2020
Changes to the blessed output file are a result of updates to atomic weights, and to fixes for how flow devices are updated (#886)
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.
Changes proposed in this pull request
This PR comes from my attempts to get repeatable output from the
combustor.cppsample in #883. I separated these changes out to avoid bloating that PR.FlowDeviceobjects is correct after a call toReactorNet::advance, and not the value at some later time that the integrator reached.timeargument toFlowDevice::massFlowRate(and equivalent methods in the various language interfaces), since getting the value at any time other than the current integrator time may not be correct.Checklist
scons build&scons test) and unit tests address code coverage