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

Adding the correct number type when parsing data #1517

Closed grdw closed 7 years ago

grdw commented 7 years ago

Fixes: #1512 Fixes: #1513 Fixes: #1517

It appeared to me that when you blank the technical_lifetime the technical_lifetime got saved to the database as '' and not as .

antw commented 7 years ago

It seems that numeric values are being saved to the asset list as strings. I'm guessing this wasn't intentional?

Before:

---
- pressure_level_index: 0
  part: pipes
  type: pe_lp
  units: 9281.578154828
  stakeholder: system operator
  building_year: 1960
  technical_lifetime: 45
  initial_investment: 130
  yearly_o_and_m_cost: 1

After:

---
- pressure_level_index: '0'
  part: pipes
  type: pe_lp
  units: '9281.578154828'
  stakeholder: system operator
  building_year: '1960'
  technical_lifetime: '45'
  initial_investment: '130'
  yearly_o_and_m_cost: '1.1'
antw commented 7 years ago

Also, updating a LES topology is returning an error due to an units input being blank:

I wouldn't object to defaulting topology nodes to have an explicit units value of 1, and showing a validation error if the user deletes the value. However I think this change in behaviour also wan't intentional?

grdw commented 7 years ago

I'm guessing this wasn't intentional?

No, it's even a bit odd looking at it. 🤔 I explicitly parse the numbers to floats or integers and than save the result. I think I might know what this could be though.

I wouldn't object to defaulting topology nodes to have an explicit units value of 1, and showing a validation error if the user deletes the value. However I think this change in behaviour also wan't intentional?

No, this wasn't intentional aswell. The thing is that units probably got saved as '' being parsed as 0 because ''.to_i == 0. I'll take a look at this aswell.

grdw commented 7 years ago

@antw

It seems that numeric values are being saved to the asset list as strings.

Turns out $.trim turns an integer or float back again to a string 😅

Graph may not have a non-numeric "units" attribute.

This was because I changed the from data('type') to data('dataType) and forgot to rename the topology form 😶

grdw commented 7 years ago

@antw if you could take a look.

antw commented 7 years ago

pressure_level_index is still being cast to a string in the DB when previously it was an int, but otherwise everything else looks good.

antw commented 7 years ago

Ahh... I spoke too soon. Strings with spaces in them – such as "system operator" and "heat producer" stakeholders – seem to have the spaces removed:

# Heat source list stakeholder:

- key: central_heat_network_dispatchable
  heat_capacity: 10.660117200536018
  units: 1
  stakeholder: heatproducer # <-----
  total_initial_investment: 100
  technical_lifetime: 30
  om_costs_per_year: 2
  marginal_heat_costs: 0.022
  distance: 1
  priority: '1'

# Gas asset list stakeholder

---
- pressure_level_index: '0'
  part: pipes
  type: pe_lp
  units: 9281.578154829
  stakeholder: systemoperator # <-----
  building_year: 1960
  technical_lifetime: 45
  initial_investment: 130
  yearly_o_and_m_cost: 1.1

Edit: And the heat source item priority is being cast to a string.

grdw commented 7 years ago

stakeholder: systemoperator # <-----

This was due to [\'\" ] instead of the correct one: [\"\']. I don't know how that extra space got in there.

The pressure_level_index missed a type, I checked and the priority of the dispatchable heat source table was also saved as a string which should have been an integer. And the demand of the technology_profile_matrix was an integer which should have been a float.

I also noticed that an attribute called dispatchable is being saved as en empty string with must run heat sources but I'm not sure why this is again. 🤔

antw commented 7 years ago

This looks good. :+1:

@grdw Could you look into the merge conflict when you have a moment? The master branch has changes to save_all.js: function change(e), but this PR removes that function entirely.

As soon as that's taken care of this can be merged (you're welcome to do it yourself if you wish).

grdw commented 7 years ago

you're welcome to do it yourself if you wish

I had to make a minor change to the technologies form to detect that someone selected a new csv but that's been taken care off. I'll merge this personally.