-
Notifications
You must be signed in to change notification settings - Fork 36
Adding HX1D and HXNTU Notebook Examples #107
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.
Looks good - most of my comments are minor things.
I made a few requests on the HX0D notebook that are also applicable to the other notebooks (I didn't repeat most of them to keep things shorter).
src/Examples/UnitModels/Operations/heat_exchanger_0D_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_0D_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_0D_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_NTU_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_NTU_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_NTU_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_NTU_testing.ipynb
Outdated
Show resolved
Hide resolved
…xamples-pse into heat-exchanger-examples
I didn't see a reason to add a separate example for the shell and tube 1D example since there seems to be significant overlap with the existing heat exchanger 1D example. However, if there's something I'm missing about it or if we just want to add a separate example for shell and tube anyway, I can do that. |
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.
Thank you @MarcusHolly for picking up this PR and adding the lumped capacitance example. it looks very nice and I just have a few comments.
src/Examples/UnitModels/Operations/heat_exchanger_lc_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_lc_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_lc_testing.ipynb
Outdated
Show resolved
Hide resolved
src/Examples/UnitModels/Operations/heat_exchanger_lc_testing.ipynb
Outdated
Show resolved
Hide resolved
…xamples-pse into bpaul4-heat-exchanger-examples
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.
One minor but important request - for the NTU example we need to clarify what MEA is to the general user. WE only need to write out the first instance in full however.
@andrewlee94 MEA is already specified in the problem statement. |
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.
@MarcusHolly Ah - it is, but it is wrong. It should be monoethanolamine.
Fixes # .
Proposed changes:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: