Move meta data from asset/market/weather classes and types to generic asset (types) #240

create-issue-branch · 2021-11-12T09:41:35Z

closes #239

- Start having our model_spec_factory get attributes it needs from GenericAsset. - Rename variables, specifically, variables that were annotated as a union of our old sensor models were named generic_asset, which was too easily confused with instances of our GenericAsset class.

…of old sensor model types

… and knowledge horizons

…n and knowledge horizons

Flix6x · 2021-11-13T10:00:05Z

Keeping track here of my process and of functionality that has/hasn't moved over:

Refactoring:

Renamed generic_asset variables to old_sensor wherever the variable was expecting a type Union[Asset, Market, WeatherSensor].
Forecasting sub-package gets its attributes from GenericAsset or Sensor instead of from old sensor models.
Planning/scheduling sub-package gets its attributes from GenericAsset or Sensor instead of from old sensor models.
API packages gets its attributes from GenericAsset or Sensor instead of from old sensor models. API package gets and sets metadata on GenericAssets and Sensors #243
UI package gets its attributes from GenericAsset or Sensor instead of from old sensor models.

Moving over old data:

Sensor units, event resolutions and knowledge horizons (fnc and par) copied from old sensors (these already had dedicated columns in both the old and new models) in a database migration. One-way transition.
Copy over all old sensor attributes that have no dedicated column in the GenericAsset model (just the display name for Market and WeatherSensor, but a lot of other attributes for Assets). These will be put in the new GenericAsset.attributes column as JSON, or in the new Sensor.attributes column as JSON.
Copy over all old sensor type attributes that have no dedicated column in the GenericAsset model (just seasonalities for Market and WeatherSensor, plus some is_consumer- and can_curtail-like attributes for Assets). Note that I'm moving these to the GenericAsset or Sensor, rather than to the GenericAssetType model.
Copy over weather correlations from the Asset model to the GenericAsset model or Sensor model.
Copy display name to both GenericAsset and Sensor.

Move over crud functionality / make tests work:

Make sure to copy over units, event resolutions, knowledge horizons and other attributes (incl. weather correlations), whenever an Asset gets created, to the already simultaneously created new sensor and generic asset.
Do this for Markets and WeatherSensors, too. API package gets and sets metadata on GenericAssets and Sensors #243
Allow JSON fields to be updated (see here). API package gets and sets metadata on GenericAssets and Sensors #243
Enable functionality to find closest Sensor (see data.services.resources.find_closest_weather_sensor.
Make sure that asset crud affects GenericAsset and Sensor crud.

…g custom

…instead have the scheduler check for an applicable market

…needs to be done jointly with moving over asset crud (which we test for) This reverts commit 56ff279.

Flix6x · 2021-11-15T10:50:36Z

I suggest we make new project tickets for the outstanding issues, fix what needs to be fixed in the scope of this PR, and merge to a dev branch for project 9 instead of to main.

nhoening · 2021-11-15T10:59:27Z

Probably a good idea. The other tickets would also live in project 9 right?

Flix6x · 2021-11-15T11:37:19Z

Exactly

…to a new class

…instead of to GenericAsset attributes

nhoening

Thanks. This is hard work, especially carrying these two models around in terminology inside the codebase at once.

I believe my comments are so small that I can attach the approval already.

flexmeasures/data/models/assets.py

flexmeasures/data/models/markets.py

flexmeasures/data/models/weather.py

flexmeasures/api/v1/implementations.py

nhoening · 2021-11-25T10:00:37Z

flexmeasures/data/models/planning/battery.py

-    device_constraints[0]["min"] = (asset.min_soc_in_mwh - soc_at_start) * (
-        timedelta(hours=1) / resolution
+    device_constraints[0]["min"] = (
+        sensor.generic_asset.get_attribute("min_soc_in_mwh") - soc_at_start


So this will raise if the Sensor does not have the necessary attributes.

Is there a good way to check? For instance, we could check the GenericAssetType. Or make one quick check if all attributes we require are present...

The planner can work without some of the attributes, such as is_pure_consumer; others it can set a default for, such as min_soc_in_mwh with default 0; yet others it cannot realistically work without, such as capacity_in_mw.

My next PR #242 introduces the has_attribute method on the Sensor and GenericAsset classes, which will be useful for this purpose. I'll make a project ticket to go over the planners and set defaults or raise where needed (see #245).

nhoening · 2021-11-25T10:01:30Z

flexmeasures/data/models/planning/charging_station.py

@@ -81,14 +81,18 @@ def schedule_charging_station(
    ) - soc_at_start * (
        timedelta(hours=1) / resolution
    )  # Lacking information about the battery's nominal capacity, we use the highest target value as the maximum state of charge
-    if asset.is_pure_consumer:
+    if sensor.generic_asset.get_attribute("is_pure_consumer"):


See my comment above about checking if required attributes are present.

nhoening · 2021-11-25T10:44:53Z

flexmeasures/data/models/forecasting/model_spec_factory.py

    else:
-        raise TypeError("Unknown generic asset type.")
+        return None


Is it okay to return None here? Are we not failing down the road?

Also, None does not fit the return typing of this function.

The return typing is Optional[Transformation], so returning None seems valid to me. And before this PR, there were also cases where we returned None within this function.

… asset (types) (#240) Refactoring: - Renamed generic_asset variables to old_sensor wherever the variable was expecting a type Union[Asset, Market, WeatherSensor]. - Forecasting sub-package gets its attributes from GenericAsset or Sensor instead of from old sensor models. - Planning/scheduling sub-package gets its attributes from GenericAsset or Sensor instead of from old sensor models. - API packages gets its attributes from GenericAsset or Sensor instead of from old sensor models. - API package gets and sets metadata on GenericAssets #243 Moving over old data: - Sensor units, event resolutions and knowledge horizons (fnc and par) copied from old sensors (these already had dedicated columns in both the old and new models) in a database migration. One-way transition. - Copy over all old sensor attributes that have no dedicated column in the GenericAsset model (just the display name for Market and WeatherSensor, but a lot of other attributes for Assets). These will be put in the new GenericAsset.attributes column as JSON, or in the new Sensor.attributes column as JSON. - Copy over all old sensor type attributes that have no dedicated column in the GenericAsset model (just seasonalities for Market and WeatherSensor, plus some is_consumer- and can_curtail-like attributes for Assets). Note that I'm moving these to the GenericAsset or Sensor, rather than to the GenericAssetType model. - Copy display name to both GenericAsset and Sensor. Move over crud functionality / make tests work: - Make sure to copy over units, event resolutions, knowledge horizons and other attributes (incl. weather correlations), whenever an Asset gets created, to the already simultaneously created new sensor and generic asset. - Do this for Markets and WeatherSensors, too. - API package gets and sets metadata on GenericAssets #243 - Allow JSON fields to be updated (see here). API package gets and sets metadata on GenericAssets #243 * Create draft PR for #239 * Db migration that copies over attributes from old data models * - In Asset.__init__, copy over attributes to GenericAsset. - Start having our model_spec_factory get attributes it needs from GenericAsset. - Rename variables, specifically, variables that were annotated as a union of our old sensor models were named generic_asset, which was too easily confused with instances of our GenericAsset class. * model_spec_factory now gets its attributes from GenericAsset instead of old sensor model types * More renaming to avoid confusion * Have db migration copy over sensor attributes: unit, event_resolution and knowledge horizons * In Asset.__init__, copy over sensor attributes: unit, event_resolution and knowledge horizons * model_spec_factory now gets event_resolution and name from Sensor * Fix tests * Factor out use of corresponding_generic_asset attribute * Factor out use of corresponding_generic_asset attribute * More renaming * Pass time series class to model configurator explicitly * Finally, model_spec_factory doesn't need the old sensor model anymore * Allow setting the collect function name for TBSeriesSpecs to something custom * In Asset.__init__, copy over additional asset attributes to GenericAsset * Planning subpackage uses sensors instead of assets * Move some simple attributes in the UI package * Refactor to stop explicitly passing the market to the scheduler, and instead have the scheduler check for an applicable market * Revert "Move some simple attributes in the UI package", because this needs to be done jointly with moving over asset crud (which we test for) This reverts commit 56ff279. * Add notes about how each attribute is to be copied from an old class to a new class * Intend to copy display_name to both GenericAsset and Sensor * Introduce Sensor attributes and copy most old model attributes there instead of to GenericAsset attributes * Adjust attribute copying in Asset.__init__ * Implement Sensor method to get an attribute * Give old sensor classes a generic_asset property * Give old sensor classes a get_attribute property * Derive Sensor class lat/lng location from GenericAsset * Get attributes from Sensor rather than from GenericAsset * Set default attributes on generic assets, too * Add clarity to method docstring Co-authored-by: Flix6x <Flix6x@users.noreply.github.com> Co-authored-by: F.N. Claessen <felix@seita.nl>

Flix6x and others added 8 commits November 12, 2021 09:41

Create draft PR for #239

638c6bb

Db migration that copies over attributes from old data models

b12fca8

model_spec_factory now gets its attributes from GenericAsset instead …

93bda19

…of old sensor model types

More renaming to avoid confusion

590a58b

Have db migration copy over sensor attributes: unit, event_resolution…

502d700

… and knowledge horizons

In Asset.__init__, copy over sensor attributes: unit, event_resolutio…

ae32c7c

…n and knowledge horizons

model_spec_factory now gets event_resolution and name from Sensor

ee74eeb

Flix6x self-assigned this Nov 12, 2021

Fix tests

b873e08

Flix6x added 11 commits November 13, 2021 11:08

Factor out use of corresponding_generic_asset attribute

f75ec8a

Factor out use of corresponding_generic_asset attribute

0278572

More renaming

b156b46

Pass time series class to model configurator explicitly

ab93262

Finally, model_spec_factory doesn't need the old sensor model anymore

734ffb6

Allow setting the collect function name for TBSeriesSpecs to somethin…

d2b7da1

…g custom

In Asset.__init__, copy over additional asset attributes to GenericAsset

df9b397

Planning subpackage uses sensors instead of assets

9bae06f

Move some simple attributes in the UI package

56ff279

Refactor to stop explicitly passing the market to the scheduler, and …

55e3d47

…instead have the scheduler check for an applicable market

Revert "Move some simple attributes in the UI package", because this …

e95c7e1

…needs to be done jointly with moving over asset crud (which we test for) This reverts commit 56ff279.

Flix6x requested a review from nhoening November 15, 2021 10:50

Flix6x marked this pull request as ready for review November 15, 2021 11:38

Flix6x added 3 commits November 16, 2021 12:59

Add notes about how each attribute is to be copied from an old class …

b4421e3

…to a new class

Intend to copy display_name to both GenericAsset and Sensor

0dd90c2

Introduce Sensor attributes and copy most old model attributes there …

414d80b

…instead of to GenericAsset attributes

Flix6x added 7 commits November 22, 2021 17:03

Adjust attribute copying in Asset.__init__

a0fca3e

Implement Sensor method to get an attribute

83dbffc

Give old sensor classes a generic_asset property

0480287

Give old sensor classes a get_attribute property

573322b

Derive Sensor class lat/lng location from GenericAsset

b1c0f22

Get attributes from Sensor rather than from GenericAsset

c3cb9da

Set default attributes on generic assets, too

2a87702

nhoening approved these changes Nov 25, 2021

View reviewed changes

Add clarity to method docstring

56eece5

Flix6x mentioned this pull request Nov 25, 2021

Planners should check for required and optional Sensor attributes more explicitly #245

Closed

Flix6x changed the base branch from main to project-9 November 25, 2021 14:05

Flix6x merged commit 58698ed into project-9 Nov 25, 2021

Flix6x deleted the issue-239-Move_meta_data_from_asset/market/weather_classes_and_types_to_generic_asset_types branch November 25, 2021 14:08

Flix6x added this to the 0.8.0 milestone Jan 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move meta data from asset/market/weather classes and types to generic asset (types) #240

Move meta data from asset/market/weather classes and types to generic asset (types) #240

create-issue-branch bot commented Nov 12, 2021

Flix6x commented Nov 13, 2021 •

edited

Flix6x commented Nov 15, 2021

nhoening commented Nov 15, 2021

Flix6x commented Nov 15, 2021

nhoening left a comment

nhoening Nov 25, 2021

Flix6x Nov 25, 2021

nhoening Nov 25, 2021

nhoening Nov 25, 2021

Flix6x Nov 25, 2021

Move meta data from asset/market/weather classes and types to generic asset (types) #240

Move meta data from asset/market/weather classes and types to generic asset (types) #240

Conversation

create-issue-branch bot commented Nov 12, 2021

Flix6x commented Nov 13, 2021 • edited

Flix6x commented Nov 15, 2021

nhoening commented Nov 15, 2021

Flix6x commented Nov 15, 2021

nhoening left a comment

Choose a reason for hiding this comment

nhoening Nov 25, 2021

Choose a reason for hiding this comment

Flix6x Nov 25, 2021

Choose a reason for hiding this comment

nhoening Nov 25, 2021

Choose a reason for hiding this comment

nhoening Nov 25, 2021

Choose a reason for hiding this comment

Flix6x Nov 25, 2021

Choose a reason for hiding this comment

Flix6x commented Nov 13, 2021 •

edited