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

771 add besmod export #789

Open
wants to merge 64 commits into
base: main
Choose a base branch
from
Open

771 add besmod export #789

wants to merge 64 commits into from

Conversation

HvanderStok
Copy link

Closes #733, #746 and #771.
Needs new BESMod version 0.6.0 RWTH-EBC/BESMod#101 and the used AixLib version is increased to 2.1.1.

Additionally, the use_conditions parameter infitration_rate is renamed to base_infiltration, as it was already named in the documentation and the parameter static_infiltration is added which is now used for the static heat load calculation with a value of 0.5.

new calculation with infiltration rate 0.5 instead of 0.2
@HvanderStok HvanderStok linked an issue Dec 9, 2024 that may be closed by this pull request
Copy link
Contributor

@FWuellhorst FWuellhorst left a comment

Choose a reason for hiding this comment

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

The feature looks really good, thanks for the hard work. I will test it locally, but I expect only these comments. Address them as you see fit. Only aspect would be backwards compatibility with base_infiltration, which we could quickly discuss with @DaJansenGit

setup.py Outdated Show resolved Hide resolved
teaser/data/output/modelica_output.py Outdated Show resolved Hide resolved
teaser/examples/e11_export_besmod_models.py Show resolved Hide resolved
teaser/examples/e11_export_besmod_models.py Outdated Show resolved Hide resolved
teaser/examples/e11_export_besmod_models.py Outdated Show resolved Hide resolved
teaser/data/output/besmod_output.py Outdated Show resolved Hide resolved
teaser/data/output/besmod_output.py Show resolved Hide resolved
teaser/data/output/besmod_output.py Outdated Show resolved Hide resolved
teaser/data/output/besmod_output.py Outdated Show resolved Hide resolved
@FWuellhorst
Copy link
Contributor

@HvanderStok Aside from the mentioned comments, this feature looks really cool. I still found some infiltration_rate cases e.g. in test_data.py, is this intended? Aside from that, incorporating the features in my branch worked like a charm. Great work!

One "issue" was that I had an extended version of TEASERThermalZone in my own code. However, the feature of this custom model is nice for others, as well, so I added it in BESMod. If users require this, they could just override the template file if they find it annoying or use own modifiers.

@HvanderStok
Copy link
Author

HvanderStok commented Dec 13, 2024

@HvanderStok Aside from the mentioned comments, this feature looks really cool. I still found some infiltration_rate cases e.g. in test_data.py, is this intended?
@FWuellhorst
infltration_rate was before directly an attribute in the ThermalZone class and was deprecated. It appears that it was not removed in these tests and also in one example. Should I refactor these and remove it finally from the ThermalZone class? The deprecation warning was stated five years ago.

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.

add BESMod export Add BESMod output
3 participants