quintel / etmoses

Online decision support tool to create local energy situations for neighbourhoods, cities and regions with a time resolution of 15 minutes created and maintained by Quintel – Not maintained
https://moses.energytransitionmodel.com
MIT License
11 stars 3 forks source link

Fix HHP units + position relative to buffer #1446

Closed grdw closed 7 years ago

grdw commented 7 years ago

This fixes the HHP units + it succesfully adds the position relative to buffer attribute to the components of the HHP.

grdw commented 7 years ago

@ChaelKruip or @antw could you test if this is what is to be expected. I'm a bit unsure what I should actually be testing for. I now only validated that the units come out correct the other end. And validate that the position relative to buffer attribute is correctly inserted. I also checked if the HHP was available in the graph. But I haven't tested anything else.

antw commented 7 years ago

I'm a bit unsure what I should actually be testing for.

  1. Add a new space heating buffer to a LES. Set number of units to 10.
  2. Add a hybrid heat pump.
  3. Save.

In the database I would expect:

  1. the hybrid heat pump components to both have units=10 (inherited from either the buffer, or the parent HHP technology)
  2. the electricity component position_relative_to_buffer to be "buffering"
  3. the gas component position_relative_to_buffer to be "boosting"

The one-line change to TechnologyProfileCalculationDecorator appears to achieve (1.) although ideally the correct number of units would also be set in the DB.

Alternatively, I think we might set no value (literally nil) for all buffer members' units (not just the HHP components), and instead set the value in the decorator prior to calculating. Either way, I think it should be consistent.

grdw commented 7 years ago

although ideally the correct number of units would also be set in the DB.

Well ideally there would be no units attribute to even store in the database. If it's null why leave the key? The decorator will decorate it back in for a component, just my 2 cents.

The weird thing why it was even in the database (since my intention was to get rid of it) was that the Javascript didn't set the units of the components in the database... turns out it was an implicit call to InstalledTechnology.new(...).to_json and the explicit call to InstalledTechnology.new(...).to_h that set the units. 😄 It automatically asumed the default amount of units (so that = 1). To stop this from happening I've overwritten those two methods in InstalledTechnology. Both methods now look like:

super.delete_if { |_,v| v.blank? }

This also saves a lot of database clutter. And it fixes the units being saved to the database.

antw commented 7 years ago

Well ideally there would be no units attribute to even store in the database. If it's null why leave the key? The decorator will decorate it back in for a component, just my 2 cents.

Indeed! I would argue this is true of all technologies which belong to a buffer – since they could inherit their number of units from the parent buffer – and not just sub-components.

I guess this also explains why position_relative_to_buffer is ends up as nil for both components. This is a problem since nil will cause the component to default to "buffering" in the calculation, whereas the gas component should be "boosting".