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

Set ASHP forced dispatch to true by default #474

Merged
merged 4 commits into from
Jan 21, 2025

Conversation

zolanaj
Copy link
Collaborator

@zolanaj zolanaj commented Jan 21, 2025

Changed

  • Set default value of force_dispatch to true in ASHP technologies

@zolanaj zolanaj requested a review from Bill-Becker January 21, 2025 03:06
Now that we default to force_dispatch=true, we can allow a lower min allowable size because we know it will dispatch a more significant amount of the heating and/or cooling load.
@Bill-Becker
Copy link
Collaborator

@zolanaj I added one significant thing: change default min_allowable_peak_capacity_fraction to 0.25, from 0.5. Let me know if you have any issue with this. My reasoning is that now that we're forcing dispatch by default, a smaller size system will produce a good amount more thermal. Also, for the LargeOffice building in particular, an ASHP system sized at 25% of peak load is serving 75%+ of the heating load.

@Bill-Becker
Copy link
Collaborator

@zolanaj I have a branch of the API with the added force_dispatch input, and I'll change the default min_allowable_peak_capacity_fraction there too which is in the clean() method of the two ASHP tech Inputs classes.

@zolanaj
Copy link
Collaborator Author

zolanaj commented Jan 21, 2025

@zolanaj I added one significant thing: change default min_allowable_peak_capacity_fraction to 0.25, from 0.5. Let me know if you have any issue with this. My reasoning is that now that we're forcing dispatch by default, a smaller size system will produce a good amount more thermal. Also, for the LargeOffice building in particular, an ASHP system sized at 25% of peak load is serving 75%+ of the heating load.

@Bill-Becker no issues with this at all, and thanks for catching the docstrings miss. I didn't alter the changelog in part because if this merges with develop before the new version is registered then there shouldn't be a need (i.e., we're editing something that we just added in the same version).

@zolanaj
Copy link
Collaborator Author

zolanaj commented Jan 21, 2025

@zolanaj I have a branch of the API with the added force_dispatch input, and I'll change the default min_allowable_peak_capacity_fraction there too which is in the clean() method of the two ASHP tech Inputs classes.

Thanks for leading the change! Let me know if I can help with the API side or review.

Copy link
Collaborator

@Bill-Becker Bill-Becker left a comment

Choose a reason for hiding this comment

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

Assuming the change of min_allowable_peak_capacity_fraction didn't break tests, I approve as is.

@Bill-Becker
Copy link
Collaborator

@zolanaj The API branch is reoptjl-v050. Can you check to make sure I am handling force_dispatch and the update to min_allowable_peak_capacity_fraction for both the models.py -> validation and communicating defaults in the /get_ashp_defaults endpoint?

@Bill-Becker Bill-Becker merged commit e2c8e3c into develop Jan 21, 2025
2 checks passed
@Bill-Becker Bill-Becker deleted the ashp-force-dispatch-true branch January 21, 2025 18:09
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.

2 participants