Skip to content
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

Add coil model, plus validation and example #1549

Merged
merged 14 commits into from
Nov 29, 2021
Merged

Conversation

AntoineGautier
Copy link
Contributor

@AntoineGautier AntoineGautier commented Nov 10, 2021

This closes #1109.

Note that the original validation model in Buildings Buildings.Fluid.HeatExchangers.Validation.WetCoilEffectivenessNTU uses Buildings.Fluid.HeatExchangers.WetCoilCounterFlow as a reference.
That reference was removed in this PR to avoid merging the latter model that does not exist in IBPSA.
When simulating the validation model, the user has no other way than looking at the referenced publication to check the model accuracy.

EDIT: For reference, I also include here the links to the review in MBL that shows the parts of the code that are duplicated from existing classes and which would benefit from a refactoring: see lbl-srg/modelica-buildings#2364 (comment) and lbl-srg/modelica-buildings#2364 (comment).

@AntoineGautier
Copy link
Contributor Author

@Mathadon This is ready on my end. Would you want to do the review?

@Mathadon
Copy link
Member

@AntoineGautier I will do the review. It may take a few days, though!

@Mathadon Mathadon self-assigned this Nov 15, 2021
@Mathadon
Copy link
Member

Just a remark: This model relies on a dew point calculation and a wet bulb temperature calculation. Both result in an algebraic loop. If we would reimplement those functions then this model could possibly require less computation time.

Copy link
Member

@Mathadon Mathadon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I did not review all model equations in detail again. I presume they are correct since the results look ok. Some minor suggestions in my review. Thanks!

IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilDryRegime.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilDryRegime.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilUARated.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilUARated.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilWetRegime.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilWetRegime.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilWetRegime.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilWetRegime.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/HeatExchangers/BaseClasses/WetCoilWetRegime.mo Outdated Show resolved Hide resolved
@AntoineGautier
Copy link
Contributor Author

@Mathadon Thanks for the review! I addressed your comments in my last commits and left unresolved the major ones for you to double check.

@Mathadon
Copy link
Member

I think that all issues are resolved. This can be merged as far as I am concerned.

@AntoineGautier AntoineGautier marked this pull request as draft November 25, 2021 14:54
@AntoineGautier
Copy link
Contributor Author

This is ready to merge once the CI tests pass and the changes in lbl-srg/modelica-buildings#2780 (review) are accepted by @mwetter.
@Mathadon Could you please approve the PR through the GH review workflow?

@mwetter mwetter marked this pull request as ready for review November 29, 2021 15:35
Copy link
Contributor

@mwetter mwetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready to merge after the CI tests pass.

@mwetter mwetter merged commit 752f6c5 into master Nov 29, 2021
@mwetter mwetter deleted the issue1109_wetCoilEpsNTU branch November 29, 2021 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

condensing cooling coil model
3 participants