spine-tools / Spine-Toolbox

Spine Toolbox is an open source Python package to manage data, scenarios and workflows for modelling and simulation. You can have your local workflow, but work as a team through version control and SQL databases.
https://www.tools-for-energy-system-modelling.org/
GNU Lesser General Public License v3.0
69 stars 17 forks source link

Embedded entity_classes (and entities) #2364

Open jkiviluo opened 10 months ago

jkiviluo commented 10 months ago

The need arose in https://github.com/spine-tools/SpineOpt.jl/issues/773 to have entity_classes that share entities from two or more entity_classes. E.g. unit class could contain members from thermal_power_plants and hydro_power_plants. Furthermore, embedded entity_classes could used when constructing multi-dimensional entity classes. E.g. unit__node class could have members both from unit__input_node and from unit__output_node.

There needs to be a way to define the type of relation for these entity_classes and how they would behave in Toolbox. This should happen through the combination of type parameter in the entity_class table and the type parameter in the entity__class__entity__class table (Spine_data_structure_for_Mopo diagram).

The first type we would need is a 'container' (better term?). It contains all members of the child classes and cannot hold members outside of the child classes. The child classes inherit the parameter definitions of the parent. Entities of the parent class cannot have parameter values. The container class can be hidden from the tree view (all edits can be done on the child class members). The 'active' status of the entity is defined by the child entity.

Later, we could have more types where these rules are changed. E.g. Parent class can have new members that are not childs. Child needs to be added to parent before it's there. Parent parameter definitions are not visible in the child entity. Parents can have parameter values (that are inherited by the child entities - but could be overridden by child). These are probably more difficult to implement, but there is no urgent use case at the moment. Their later addition should be considered in the implementation though.

manuelma commented 10 months ago

I think the starting point needs to be the concept of abstract class, with the following properties:

  1. The abstract class cannot have entities of its own.
  2. It can be subclassed by a regular (concrete) entity class.
  3. It can have parameter definitions, which are inherited by the concrete subclasses. In other words, we can specify values of the abstract class parameters for entities of the subclass.
  4. It can be used as dimension in any concrete entity class. Then, when creating entities, we can select for that dimension an entity of any of the subclasses.

I think this is the way it's done in object oriented programming. We can use it as a starting point - I think it already satisfies our requirements from https://github.com/spine-tools/SpineOpt.jl/issues/773

jkiviluo commented 10 months ago

Right, I think that's pretty much the same as my 'container' class. I was trying to generalize for the future where we might have needs for other similar classes but with different rules (like that there are parents with parameter values and those values are inherited by the child unless the child overwrites that parameter value - I think is the data structure used by powersystems.jl).

manuelma commented 10 months ago

Yes, it's your container class - they call it abstract in OOP. I think we can also call it abstract class to immediately hint that it cannot have entities.

DillonJ commented 10 months ago

It sounds like this could be useful, but I think we'd need a full view of what this would mean and how we would use.

@jkiviluo The original motivation here was to subordinate unit__input_flow and unit__output_flow to a higher-level abstract class, unit__node. I'm wondering if your example of thermal_power_plants and hydro_power_plants is just arbitrary, or is this how you would see the functionality being used? I'm wondering because this would mean that specific technologies are built into the data structure. This could be convenient but we'd need to do it in a way that leaves the generic case open. I know the abstract class can't have entities, but you would at least need to have a generic sub-class that allows a user to create arbitray technologies using nodes and units in the usual way and not force users to create a new sub_class to create something not "out of the box", which is a key strength of SpineOpt. A user could of course, create a sub-class eventually for their specific technology if they wanted. I guess it sounds like the original archetypes idea but generalised which is good.

Also, as we have seen, the additional danger of writing specific things into the data structure is that users often have preferences for how things are modelled. Take our various discussions on storages for example - users have strong preferences for modelling them this way or that way... the existing flexible structure caters to a users' preference (at the cost of perhaps not proving guidance where no preference exists) but we should always make sure it's always as easy as it is to do now - to do flexible arbitrary things - and make sure they are not considered as special cases that become harder to implement.

As I've mentioned a few times already - the other possibilty for providing these conveniences (both the example of unit_node and thermal_power_plants and hydro_power_plants is to have a higher level tool that does this and translates it into the fundamental SpineOpt structure. A very comprehensive pre-processing if you will, that sits outside of SpineOpt and can better managed and maintained.

I feel like we should try and come to some sort of view on how we want to manage this convenience/flexibility issue at a high level and this would probably resolve a lot of these issues. A higher-level design I guess.

jkiviluo commented 10 months ago

your example of thermal_power_plants and hydro_power_plants is just arbitrary, or is this how you would see the functionality being used?

It's of course arbitrary in the context of SpineOpt. But this is a Toolbox issue and Toolbox needs to support tools that have such data structures.

manuelma commented 10 months ago

The immediate application for SpineOpt is to define an abstract class unit__node of which unit__from_node and unit__to_node are concrete subclasses. That would have two benefits:

  1. A lot of parameter definitions (maybe all of them) that are common to unit__from_node and unit__to_node could be associated to unit__node instead (because of point 3 above https://github.com/spine-tools/Spine-Toolbox/issues/2364#issuecomment-1770983004), saving space, lines in the SpineOpt template, and gaining a little bit of elegance.
  2. We could create unit__node___unit__node as a concrete class, with dimensions unit__node and unit__node (the abstract class). As per point 4 above https://github.com/spine-tools/Spine-Toolbox/issues/2364#issuecomment-1770983004), we could then create unit__node___unit__node entities with either a unit__from_node or a unit__to_node in each position. This would reduce the number of classes needed and simplify life for the user as they will not need to care if the flows are from or to when defining ratios.
jkiviluo commented 10 months ago

The link above is to a related issue that helps to create entities that distinguish between subclasses (since there can be entities with exactly same names in different subclasses).

manuelma commented 10 months ago

So we need to deal with something I believe @DillonJ was pointing out somewhere, related to the "ambiguity" of unitnodeunitnode. The issue arises when I have, say, a unitfrom_node and a unitto_node between the same unit and node. If I then want to create a unitnodeunitnode between my unitfrom_node and my unitto_node, it is not enough to list the names of the four zero-D entities involved. One would also need to indicate the concrete class of each of the two unit-node pairs.

That can be nasty, so I propose that instead, one creates a unitnodeunit__node by specifying the names of the two 2-D entities involved.

The above means that users need to start to pay more attention to the name of multi-D entities. I believe this can be a good thing in general...

jkiviluo commented 10 months ago

When adding entities manually, then https://github.com/spine-tools/Spine-Toolbox/issues/2370 should work. However, when e.g. copy-pasting parameter data, then the entity names are probably the best solution. At the moment, we allow same entity names in different entity classes. Should that be disallowed from entities that share a common abstract entity_class?

Other option could be to switch to unit__to_node and node__to_unit as discussed from https://github.com/spine-tools/SpineOpt.jl/issues/773#issuecomment-1778947388 onward. I'm starting to like that idea (in case you can wrap both in the same abstract class).

manuelma commented 10 months ago

...same entity names in different entity classes. Should that be disallowed from entities that share a common abstract entity_class?

Yes.

Other option could be to switch to unit__to_node and node__to_unit

Looks like that works but I wouldn't want to rely on something like that. It's too limiting - only works if you don't need more than two classes, but what if we also wanted to have, say, unit__fuel_node or something like that (maybe bad example, but you get the idea).

jkiviluo commented 10 months ago

Right, it needs to work on unit__x_node type of situations too and we should disallow same entity names in the same abstract class.

unit__to_node and node__to_unit can still be considered in the context of SpineOpt for other merits that system could have (but not the discussion here, since here we need to make the generic case to work).

DillonJ commented 10 months ago

If we were going that way, then unitnode and nodeunit would work surely, but no harm having the "to" for extra clarity?

jkiviluo commented 10 months ago

I think so too.

manuelma commented 10 months ago

Part of this has been done via https://github.com/spine-tools/Spine-Database-API/issues/299

But the title and description of this issue is more ambitious and it looks it can take a while before we can close this one. I guess it's okay to just keep it open as a reminder of where we might want to go in the future?

jkiviluo commented 10 months ago

Great!

Yes, let's keep it open. I'll put a later milestone.