-
Notifications
You must be signed in to change notification settings - Fork 36
Adding PFR and StoichiometricReactor Notebook Examples #84
Conversation
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.
I haven't look at the stoichiometric reactor notebook yet, but I am guessing many of the comments for the PFR notebook will apply there too.
src/Examples/UnitModels/Reactors/plug_flow_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/plug_flow_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/plug_flow_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/plug_flow_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/plug_flow_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/plug_flow_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/plug_flow_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/plug_flow_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
Thank you @andrewlee94 for your comments, I will review and update all three notebooks for any changes. |
@andrewlee94 I've made the suggested changes in all three rate-reactor notebook examples, including adding the two links. Additionally, I reformulated the initial plug-flow reactor problem which solves correctly now. |
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.
I did a quick read through of the stoichiometric reactor example this time and only had a few comments. Most of them are imports I don't think you need, plus a couple of things that don't make much sense for a stoichiometric reactor, but were left over from the CSTR/PFR example.
src/Examples/UnitModels/Reactors/stoichiometric_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/stoichiometric_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/stoichiometric_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/stoichiometric_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/stoichiometric_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Reactors/stoichiometric_reactor_testing.ipynb
Outdated
Show resolved
Hide resolved
@andrewlee94 I cleaned up the notebooks and updated the PR |
@MAZamarripa @jghouse88 This PR is awaiting an additional review. Please let me know if you will have time to review this, or if there is another reviewer I may add. Thanks! |
"2021-12-13 13:43:04 [INFO] idaes.init.fs.R101.control_volume.reactions: Initialization Complete.\n", | ||
"2021-12-13 13:43:04 [INFO] idaes.init.fs.R101.control_volume: Initialization Complete\n", | ||
"2021-12-13 13:43:04 [INFO] idaes.init.fs.R101: Initialization Complete: optimal - Optimal Solution Found\n" | ||
"2022-04-13 14:49:03 [INFO] idaes.init.fs.M101.reagent_feed_state: Starting initialization\n", |
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.
From what I know, we are not supposed to include the output in the notebooks. @andrewlee94 what is the policy on this?
"m.fs.R101.conversion.fix(0.5)\n", | ||
"\n", | ||
"m.fs.R101.heat_duty.setub(0*pyunits.J/pyunits.s) # heat duty is only used for cooling\n", | ||
"\n", |
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.
Is an upper bound on heat duty necessary? At first, I thought you were fixing the heat duty, because this is where you are specifying the degrees of freedom. Only after I looked at it carefully, I realized that you are setting an upper bound. If it is not necessary, I suggest you to remove this. But if it is necessary, move the line to a separate cell to avoid confusion.
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.
The upper bound is necessary for convergence. I split this line into a separate cell, with text explanation.
"m.fs.R101.heat_duty.setub(0*pyunits.J/pyunits.s) # heat duty is only used for cooling\n", | ||
"\n", | ||
"m.fs.R101.length.fix(1*pyunits.m)" | ||
] |
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.
I don't know what I'm missing here. The rate of reaction equation requires the volume of the fluid. You are specifying the length here, but I didn't see the cross-sectional area specification anywhere. How is the volume calculated then?
"m.fs.R101.length.unfix()\n", | ||
"m.fs.R101.length.setlb(0*pyunits.m)\n", | ||
"m.fs.R101.length.setub(5*pyunits.m)\n", | ||
"\n", |
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.
This goes back to my earlier question. From the notebook, it is not clear how the length and the volume are related. Here, you are letting both the volume and length free. It would help if there is an explanation on how these two properties are related.
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.
To address your questions in this comment and the comment above, reaction extent (conversion), reaction rate (k_rxn) and the reactor volume are interdependent through the performance equation in the thermophysical property paragraph. The performance equation is an internal model constraint. By fixing the conversion and inlet temperature, we determine the required reactor volume. The volume is defined as volume = length*area
, and therefore specifying length for our square problem determines the required cross-sectional area.
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.
Looks good to me.
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.
I only did a quick skim, so I will trust that my prior reviews were sufficient.
Fixes # .
Proposed changes:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: