Fix for batteries and vehicles when using unit multipliers #2137
Conversation
| elsif hpxml_bldg.vehicles.size > 0 | ||
| # Same as battery issue above | ||
| return |
There was a problem hiding this comment.
I believe this should have been removed in #2113.
joseph-robertson
changed the title
|
It's possible there is something else going on with using unit multipliers with batteries. The discrepancies can be pretty non trivial. @jmaguire1 is going to ask @rchintala13. It's possible we should be adjusting the number cells in series and not just the number of strings in parallel. |
shorowit
left a comment
shorowit
left a comment
There was a problem hiding this comment.
Some initial comments/thoughts.
ReportSimulationOutput/measure.rb
Outdated
| vars = ['Other Equipment Electricity Energy'] | ||
| keys = resilience.variables.select { |v| v[2] == vars[0] }.map { |v| v[1] } | ||
| batt_loss = get_report_variable_data_timeseries(keys, vars, UnitConversions.convert(1.0, 'J', 'kWh'), 0, resilience_frequency) | ||
| batt_loss = batt_loss.map { |x| x * unit_multiplier } |
There was a problem hiding this comment.
Hmm, I don't see the unit multiplier used anywhere else in the reporting measure. It's possible that this situation is unique, but I wonder if we can switch from the output variable we're currently using to the equivalent output meter that already incorporates the unit multiplier?
More specifically, I noticed that we use an output meter here for the battery losses when calculating the end use, but then use an output variable here for the battery losses when calculating the resilience metric. Maybe the latter just needs to be like the former?
There was a problem hiding this comment.
See bugfix explanation in #2140 (comment).
ReportSimulationOutput/measure.rb
Outdated
| return i / Float(ts_per_hr) if batt_soc_kwh <= 0 | ||
| return i / Float(ts_per_hr) if batt_soc_kwh.abs <= Constants::Small |
There was a problem hiding this comment.
I don't understand this change. If it's a large negative number, it no longer returns? Why is it not batt_soc_kwh <= Constants::Small if it's just a floating point issue?
There was a problem hiding this comment.
I also don't see any diffs for CI results, why is that?
There was a problem hiding this comment.
Oops, I don't know why I put the .abs on there.
Can batt_soc_kwh even be negative?
There was a problem hiding this comment.
Oh, right, it surely can't. So the code was really just checking for zero SOC, but being extra safe by looking for <= 0.
Pull Request Description
This PR fixes several issues:
eff_discharge_powerbyunit_multiplierChecklist
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