-
Notifications
You must be signed in to change notification settings - Fork 27
Support modeling batteries for whole SFA/MF buildings #2129
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
base: unit-fuel-meters
Are you sure you want to change the base?
Conversation
…electric storage.
workflow/tests/util.rb
Outdated
| elsif hpxml_bldg.vehicles.size > 0 | ||
| # Same as battery issue above | ||
| return | ||
| # return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we missed removing this in #2113?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@joseph-robertson I don't think this is going to work correctly. Because the battery operation scheme is |
Right. That's what I'm doing in this PR. |
Oh, wow. Sorry I guess I should have looked at the implementation before commenting. Glad to see you're aware of what it would take! |
…tors or batteries/vehicles.
workflow/tests/util.rb
Outdated
| abs_delta_tol = 10 | ||
| abs_frac_tol = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that previously we weren't ever checking batteries with unit multipliers. This is because in our tests we only use unit multipliers along with whole SFA/MF building simulations. But whole SFA/MF building simulations with batteries were returning (based on the limitation) before checking unit multiplier results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, I'm pretty sure that unit multipliers aren't working correctly for homes with a battery. At least for base-battery-scheduled-power-outage.xml. When NumberofUnits=1, resilience hours are 1.254. When NumberofUnits=2, resilience hours are 31.691.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shorowit had the idea to test unit multipliers, for technologies that aren't supported in whole SFA/MF buildings, by using a single building with multiplier of 10 instead of 2 buildings with multipliers of 5. Although, maybe the need for this approach may no longer be needed once this PR is merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| next if object.to_AdditionalProperties.is_initialized | ||
| next if object.to_EnergyManagementSystemOutputVariable.is_initialized | ||
|
|
||
| vars_by_key = get_object_outputs_by_key(model, object, EUT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice to (re)use the get_object_outputs_by_key method, but not so nice that there's some extra special stuff below.
|
I'm seeing that changing from TrackFacilityElectricDemandStoreExcessOnSite to TrackMeterDemandStoreExcessOnSite leads to discrepancy in End Use: Battery when there is also a generator in the building (e.g., I'm not totally sure what to make of this. Perhaps in the TrackFacilityElectricDemandStoreExcessOnSite case the ELCD accounts for on-site generation produced by the generator, but in the TrackMeterDemandStoreExcessOnSite case the ELCD ignores it? What should the path forward be here? To continue with using TrackFacilityElectricDemandStoreExcessOnSite, or switch to TrackMeterDemandStoreExcessOnSite, for single unit simulations? Edit: I took the TrackMeterDemandStoreExcessOnSite path. c888e49 |
| # Since custom meter cannot reference other custom meters, copy | ||
| # all custom meter key/variable groups to this custom meter. | ||
| key_vars = custom_unit_meter.keyVarGroups |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where it would be nice to allow custom meter to reference other custom meters.
Pull Request Description
Addresses #1499. Change the battery operation scheme to TrackMeterDemandStoreExcessOnSite, and track unit electricity meters.
Checklist
Not all may apply:
EPvalidator.sch) has been updatedopenstudio tasks.rb update_hpxmls)HPXMLtoOpenStudio/tests/test*.rband/orworkflow/tests/test*.rb)openstudio tasks.rb update_measureshas been run