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

Issue3063 District DHW #3525

Merged
merged 211 commits into from
Jan 6, 2024
Merged

Issue3063 District DHW #3525

merged 211 commits into from
Jan 6, 2024

Conversation

dhblum
Copy link
Contributor

@dhblum dhblum commented Sep 16, 2023

This is for #3063. In particular, it:

  • Implements component level domestic hot water models for two sources: 1) district heat exchanger and auxiliary electric heat and 2) water-to-water heat pump with storage tank.
  • Implements component level domestic hot water models for fixture load(s) and thermostatic mixing valve.
  • Adds a building load option using the Building Time Series load model and heat pump-based ETS model that includes the heat pump with storage tank DHW generation source. Led to creation of associated partial ETS class that is extended with the original DHW modeling approach in one model, and the new one in another.
  • Various updates to model names and documentation.
  • Commits unit test results.

Still todo:

  • Review
  • Update release notes
  • Check openmodelica unit tests pass with 1.22 update
  • Add optimica unit tests to exclusion list

Dre Helmns and others added 30 commits June 15, 2022 17:59
This resolves errors in DirectHeatExchangerWaterHeaterWithAuxHeat caused by medium models not matching across components.
In DirectHeatExchangerWaterHeaterWithAuxHeat, the variable havePEle is already declared in its parent model PartialFourPortDHW.
This commit exposes more errors.
This is done by manually deleting and remaking connectors from souDcw in the graphic view.
@dhblum
Copy link
Contributor Author

dhblum commented Dec 16, 2023

@mwetter I've updated the port locations and also the icons for the heat pump and heat pump with DHW subsystems. As far as I know, there's still the issue of simulating some models in Optimica. But let me know if there's anything else needed that you see to make this ready to merge.

Copy link
Member

@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.

@dhblum : Please go ahead and merge it when the release notes are updated (which I think you did today).
Regarding your note about issues with some models in Optimica, I believe these are old issues, right? I am asking as I don't see a new entry in conf.yml

@dhblum
Copy link
Contributor Author

dhblum commented Dec 18, 2023

Thanks @mwetter. Yes I added release notes today. I have not added the optimica errors to conf.yml yet. The openmodelica tests will also fail if the unit test image isn't updated to 1.22.0. I can either add to the conf.yml until #3603 is merged, or wait for #3603 to be merged before merging this one.

@mwetter
Copy link
Member

mwetter commented Dec 19, 2023

@dhblum : I suggest you don't add the openmodelica entries in conf.yml. I hope to have fixed the latest timeouts in the PR of #3603. The PR should run this morning as there is only one more in the queue ahead.

@dhblum
Copy link
Contributor Author

dhblum commented Dec 19, 2023

@mwetter Sounds good, I'll hold off until #3603 is merged.

@dhblum
Copy link
Contributor Author

dhblum commented Dec 21, 2023

@mwetter Unit tests for openmodelica are failing still after merging the latest master with update to OM 1.22.0, with a failure of initialization for one model. But, when I run that model with OM 1.22.0 locally it works fine. The only difference I can see is that I run on Ubuntu 20 while the travis unit test runs with a Ubuntu 22 (Docker). Do you have experience that the same OM version but on different OS versions can cause a problem?

@mwetter
Copy link
Member

mwetter commented Dec 21, 2023

Different OS and OS versions may yield slightly different results, such as due to different compiler versions and/or libraries. I suggest you put it on the exclude list for now, and when we update to the new OpenModelica version, revisit it.

@dhblum
Copy link
Contributor Author

dhblum commented Dec 21, 2023

Ok I see, thanks. Sounds reasonable, I'll add it to the exclude list.

@dhblum
Copy link
Contributor Author

dhblum commented Jan 4, 2024

@mwetter When you typically merge to master do you have any preference for squash merges or not?

@mwetter
Copy link
Member

mwetter commented Jan 4, 2024

Either is fine.

@dhblum dhblum enabled auto-merge January 5, 2024 17:57
@dhblum dhblum merged commit 259f6a4 into master Jan 6, 2024
2 checks passed
@dhblum dhblum deleted the issue3063_dhwLoaDisHea branch January 6, 2024 15:10
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.

3 participants