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.

Sign up for GitHub

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

Jump to bottom

Initialize tank temperature #186

Merged
merged 7 commits into from Dec 11, 2023
Merged

Initialize tank temperature #186

merged 7 commits into from Dec 11, 2023

Conversation

spahrenk
Copy link
Contributor

@spahrenk spahrenk commented Dec 5, 2023 •
edited

Description

  • Allows specification of an initial temp temperature that differs from the setpoint. The initial temperature can be specified in three different ways:
  1. Using preset: assign a temperature initialTankT_C and set hasInitialTankTemp = true.
  2. Using file: create an entry initialTankTemp in the model specification file.
  3. Using test profile: add an entry initialTankT_C in testInfo.txt.
    Note that method 3 generally overrides methods 1 and 2.
  • This addresses a need to assign an initial, independent tank temperature for the AquaThermAire mode.
  • Each method was tested manually. No new tests have been added to the framework.

Author Progress Checklist:

  • Open draft pull request
    • Make title clearly understandable in a standalone change log context
    • Assign yourself the issue
    • Add at least one label (enhancement, bug, or maintenance)
    • Link the issue(s) addressed by this PR (under "Development" in the sidebar menu)
  • Make code changes (if you haven't already)
  • Self-review of code
    • My code follows the style guidelines of this project
    • I have added comments to my code, particularly in hard-to-understand areas
    • I have only committed the necessary changes for this fix or feature
    • I have made corresponding changes to the documentation
    • My changes generate no new warnings
    • I have ensured that my fix is effective or that my feature works as intended by:
      • exercising the code changes in the test framework, and/or
      • manually verifying the changes (as explained in the the pull request description above)
    • My changes pass all local tests
    • My changes successfully passes CI checks
    • Add any unit test for proof and documentation.
    • Merge in main branch and address resulting conflicts and/or test failures.
  • Move pull request out of draft mode and assign reviewers
  • [] Iterate with reviewers until all changes are approved
    • Make changes in response to reviewer comments
    • Merge in main branch and address resulting conflicts and/or test failures.
    • Re-request review in GitHub

Reviewer Checklist:

  • Read the pull request description
  • Perform a code review on GitHub
  • Confirm all CI checks pass and there are no build warnings
  • Pull, build, and run automated tests locally
  • Perform manual tests of the fix or feature locally
  • Add any review comments, if applicable
  • Submit review in GitHub as either
    • Request changes, or
    • Approve
  • Iterate with author until all changes are approved

@spahrenk spahrenk self-assigned this Dec 5, 2023
@spahrenk spahrenk marked this pull request as ready for review December 5, 2023 18:06
@spahrenk spahrenk mentioned this pull request Dec 6, 2023
Closed
33 tasks
nealkruis
nealkruis requested changes Dec 9, 2023
View reviewed changes
test/testUtilityFcts.cc Outdated
Comment on lines 300 to 302
else if (modelName == "BasicIntegrated") {
hpwhModel = HPWH::MODELS_basicIntegrated;
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this change relevant?

src/HPWH.cc Outdated
@@ -3290,6 +3290,8 @@ int HPWH::HPWHinit_file(string configFile) {
//some variables that will be handy
int heatsource,sourceNum,nTemps,tempInt;
std::size_t num_nodes = 0, numHeatSources = 0;
bool hasInitialTankTemp = false;
double initalTankT_C = 20.;
Copy link
Member

Choose a reason for hiding this comment

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

20 C seems low...this should maybe be closer to 120 F.

src/HPWHpresets.cc Outdated
@@ -411,6 +411,9 @@ int HPWH::HPWHinit_presets(MODELS presetNum) {

heatSources.clear();

bool hasInitialTankTemp = false;
double initialTankT_C = 20.;
Copy link
Member

Choose a reason for hiding this comment

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

Same here: 120 F.

src/HPWHpresets.cc Outdated
@@ -515,7 +518,7 @@ int HPWH::HPWHinit_presets(MODELS presetNum) {

else if (presetNum == MODELS_StorageTank) {
setNumNodes(12);
setpoint_C = 800.;
setpoint_C = F_TO_C(127.);
Copy link
Member

Choose a reason for hiding this comment

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

I think I was confused before. I don't think the setpoint should change (assuming 800 was chosen to do something to disable the heat source controls). Instead, the initial temperature for this preset should be set to 127F.

@nealkruis nealkruis self-requested a review December 11, 2023 16:15
@nealkruis nealkruis merged commit e026f15 into master Dec 11, 2023
2 checks passed
@nealkruis nealkruis deleted the initialize-tankT branch December 11, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nealkruis nealkruis nealkruis approved these changes

Assignees

@spahrenk spahrenk

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@spahrenk @nealkruis