spine-tools / SpineOpt.jl

A highly adaptable modelling framework for multi-energy systems
https://www.tools-for-energy-system-modelling.org/
GNU Lesser General Public License v3.0
53 stars 13 forks source link

Equations and DB naming conventions and style guide [REPLACEMENT ISSUE] #55

Closed spine-o-bot closed 3 years ago

spine-o-bot commented 3 years ago

The original issue

Id: 53
Title: Equations and DB naming conventions and style guide

could not be created. This is a dummy issue, replacing the original one. It contains everything but the original issue description. In case the gitlab repository is still existing, visit the following link to show the original issue:

TODO

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 29, 2018, 14:19

changed the description

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 29, 2018, 14:20

changed the description

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 29, 2018, 14:21

changed the description

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 29, 2018, 14:21

changed the description

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 29, 2018, 20:35

Good one, the sooner we settle this the better, I believe.

So I agree with your initial suggestions except that I don't see hyphens working, they'll just be interpreted as the subtraction operator in julia.

I believe The only caracter that's not an operator appart from the underscore is the exclamation point !

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 06:32

Below an overview of what we currently have done in the model formulation (I think this is quite readable):

parameter name:

format: p_[Parameter name]

allowed characters: Uppercase + lowercase, where uppercase used for the first letter of a new segment

example: p_MaxRatioOutputInputFlow

class name:

format: plurals

allowed characters: Uppercase + lowercase, where uppercase used for the first letter of a new segment

example: Units, UnitGroups

relationship class:

format: related class names

allowed characters: Uppercase + lowercase, where uppercase used for the first letter of a new segment

example: NodesCommodities, UnitGroupsUnits

Note that class names and relationship classes both translate into sets in Spine (classes are sets containing single elements, relationship classes translate into sets of tuples), so it would be good to use the same conventions for both.

variable name:

format: v_[Varameter name]

allowed characters: Uppercase + lowercase, where uppercase used for the first letter of a new segment

example: v_Flow, v_UnitsOnline

index:

format: short format name for class, ideally the first character of the class name (g appended for group)

allowed characters: lowercase only

example: u (unit), ug (unit group)

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 06:39

We're clearly at war here. It's the good old discussion about what is more readable, lower_case or UpperCase. And what makes more sense, singular or plural.

I'm in favor of lower_case and singular, but that's just because it's the standard for julia functions and variables. UpperCase is reserved to types, classes, and package names.

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 06:56

We had this discussion in Helsinki and agreed on lower case. I've attached the meeting memo :-) Memo.pdf

This is what was noted:

Syntax (long discussion on the benefits and drawbacks of modern environments that can highlight syntax): o Multi-word object and relationship names: lowercase (but try to keep them single word) o Objects: all small caps, signifies relationship o Parameters: descriptive enough, capital first letter? Could we use in parameters? o Indexes: Unit u, time t, forecast f, node n, sample s, commodity c, rest verbose o Equations: express meaning (can be longer, can use _) o Variables: relatively short, still descriptive o o Different colors for variables, parameters, indices (for those who use proper environment) o Break the line before the sign oo

  • fuelUse
    =E=
  • sum (u in unit,
  • power
  • co_efficient
  • heat )

There are other considerations than just readability. The reasoning behind lower case is that if we allow uppercase, it's impossible to regulate and be consistent - it would make it much more difficult to remember a parameter name precisely and would lead to errors.

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 06:58

Normally, I don't have a strong opinion on these type of issues :).

But here, I think it is not just about being able to read the separate parts (lower_case or UpperCase). Purely on the basis of readability, I would say both are fine, but my preference would also go to lower_case.

However, since we are already using the underscore for making clear whether something is a parameter or a variables (p_parametername, v_variablename, and potentially to identify the different classes in a relationship class), I am not really a fan of adding underscores for separating words as well.

Regarding singular versus plural. Here I think the equations just read better using plurals as these are typically sets: e.g., for u in Units

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:01

@Poncelet Yes, we agreed in Helsinki that we would use single words where possible.

Let's take the example of a unit group. Do we call it unit_group or unitgroup? The former is more readable but the second works better when we want to create relationships like, for example, unit_unitgroup. If we had unit_unit_group it would be ambiguous.

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:03

@Poncelet if we use p_ for all parameters, we have to bear in mind that this is how they will be in the database also - which may be a little tedious? Other than that, I don't have a strong view - except that I think we shouldn't use uppercase

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:05

Do we need the v_ and p_ prefix to tell apart variables from parameters? The fact that parameters are 'functions' already introduces some distinctive traits julia level. They will be highlighted differently by most editors (Atom, Sublime). Variables are accessed by [] whereas parameters are accessed by ()

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Aug 30, 2018, 07:07

There is also issue #31, but I guess the discussion is now here.

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:07

Yes, we agreed in Helsinki that we would use single words where possible.

True, but there was no model at that point. I don't think we can reduce MaxRatioOutputInputFlow to a single word :)

Let's take the example of a unit group. Do we call it unit_group or unitgroup? The former is more readable but the second works better when we want to create relationships like, for example, unit_unitgroup. If we had unit_unit_group it would be ambiguous.

Exactly, that is why I transitioned to UnitGroup. Than, you can still have Unit_UnitGroup, which is umambiguous.

To sum up my idea: for readability, I think both uppercase and lowercase are fine. However, sometimes we want to explicitly connect different things (e.g., Unit_UnitGroup), and that is the most clear using an underscore. But in that case, we cannot use underscores anymore purely for readability as that would lead to ambiguity. That is why it is useful to be able to separate parts of longer words using UpperCase...

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:07

@Poncelet we have in general agreed to use singular already... e.g. unit, unitgroup, commodity in the database design for example. I prefer the singular - it would lead to ambiguity if we started using plurals at this stage - by contrast we would achieve nice consistency if we stick with singular

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:09

The problem is that when you say "both uppercase and lowercase are fine." you will never remember when trying to reference a parameter which it was you used initially and you will constantly have to be looking it up which will be a total pain. Whatever we do, we need a convention and need to stick to it

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:09

Is it too bad to use the exclamation point to indicate relationships?

unit!unit_group

In this way we could reserve the underscore for separating words...

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:10

we have in general agreed to use singular already... e.g. unit, unitgroup, commodity in the database design for example. I prefer the singular - it would lead to ambiguity if we started using plurals at this stage - by contrast we would achieve nice consistency if we stick with singular

But there is not yet consistency. Plurals are e.g., in the model formulation... I just feel the plural is a better description of what it is (a set of multiple units).

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:11

The problem is that when you say "both uppercase and lowercase are fine." you will never remember when trying to reference a parameter which it was you used initially and you will constantly have to be looking it up which will be a total pain. Whatever we do, we need a convention and need to stick to it

But they are fine in very distinct places. Underscore only for p and v and for relationships. Readability using UperCase. Not too hard, right?

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:11

Plural gets ugly in multiD relationships units_commodities_nodes

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:11

Is it too bad to use the exclamation point to indicate relationships?

My personal feeling, yes, this does not look nice :)

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:13

Plural gets ugly in multiD relationships units_commodities_nodes

But still, it is a set containing all units, linked to all commodities and all nodes...

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:13

Are you sure the exclamation would work in Julia? It works for me!

@Poncelet but what you use in the model you use in the database and vice versa - the conventions apply right across. All the names pass through directly from the DB into the model, so we can't define standards for each independently.. and even if we could, it wouldn't be good practice to have different conventions in difference places.

I'm with @manuelma on the singular... it just gets messy.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:13

My personal feeling, yes, this does not look nice :)

Ok.. :(

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Aug 30, 2018, 07:14

! might not work in the DB side?

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:14

Also, for the exclamation, I think it will not work in other languages...

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:15

What about double underscore for relationships

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Aug 30, 2018, 07:15

I'm also for singular. Manuel convinced me when building the first versions of the db.

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:15

@Poncelet but what you use in the model you use in the database and vice versa - the conventions apply right across. All the names pass through directly from the DB into the model, so we can't define standards for each independently.. and even if we could, it wouldn't be good practice to have different conventions in difference places

Yes, I agree we need to be consistent. I just think at the moment, we are not consistent yet, so the discussion on which style to use is open...

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:17

I guess I have to surrender on the singular, although the arguments did not convince me :)

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Aug 30, 2018, 07:17

While I like p and v in the code (although it's not really needed as Manuel said), I think they are just too ugly in the DB and thus we shouldn't use them.

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:18

While I like p and v in the code (although it's not really needed as Manuel said), I think they are just too ugly in the DB and thus we shouldn't use them.

What do you suggest instead? I was not a fan of it in Espoo, but I now like how easy to read it is.

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Aug 30, 2018, 07:19

Just parameter name (capacity) and variable name (flow) as they are.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:19

maybe let's just use v_ for variables and leave parameters alone. Variables don't go in the database do they?

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:20

At present, we are using singular class and relationship names.

function constraint_TransCap(m::Model,v_Trans)
    @constraint(m, [con in connection(), i in node(),j in node(), t=1:number_of_timesteps(time="timer"); [con,i,j] in connection_node_node()],
        + (v_Trans[connection=con,node1=i,node2=j,t=t])
        <=
        + p_TransCapAvFrac(connection=con,node1=i,node2=j,t=t)
            * p_TransCap(connection=con)
    )
end
spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:20

Variables end up in the database as well. And the v_ probably is annoying for post-processing.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:21

Variables end up in the database as well. And the v_ probably is annoying for post-processing

of course they do, sorry.

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Aug 30, 2018, 07:22

Testing...

toolongparametername_relatedtoanother
tooLongParameterName_relatedToAnother
TooLongParameterName_RelatedToAnother
too_long_parameter_name__related_to_another

When I look at that list, I have to say the last one is clearly easiest to read.

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:22

As @manuelma says, because each parameter name is a function, you get syntax highlighting that distinguishes variables from parameters. Capture2

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:24

Just parameter name (capacity) and variable name (flow) as they are.

Yes, in the code there would be syntax highlighting, but in the model formulation, there wouldn't be...

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:24

@juha_k double__underscore... interesting idea...

Another, probably terrible idea is to use 2 to identify relationships. unit_2_unit_group

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:25

I kinda like the 2 but remove the underscores in that case:

unit2unit_group

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Aug 30, 2018, 07:25

credit to @manuelma on __

_2_
(or 2) is bit ugly, especially in the DB.
spine-o-bot commented 3 years ago

In GitLab by @manuelma on Aug 30, 2018, 07:26

too_long_parameter_name__related_to_another

this is nice

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:27

@juha_k Agreed that the last one in your list is easiest to read and would be my preference... also, these two illustrate the hazard of allowing caps... you would (150% of the time) run into this issue

tooLongParameterName_relatedToAnother
TooLongParameterName_RelatedToAnother
spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:27

too_long_parameter_name__related_to_another

In most cases, you would use either _ or (e.g., fix_ratio_output_input_flow or node__commodity), but I guess the difference is relatively clear.

spine-o-bot commented 3 years ago

In GitLab by @jkiviluo on Aug 30, 2018, 07:29

Would we be settled then on that count? Unless we get others involved...

spine-o-bot commented 3 years ago

In GitLab by @Poncelet on Aug 30, 2018, 07:29

I'm still not a fan of not having a distinction between variables and parameters. So no model description? Different syntax to use in papers, etc...

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:30

I'm good with too_long_parameter_name__related_to_another

spine-o-bot commented 3 years ago

In GitLab by @DillonJ on Aug 30, 2018, 07:30

@Poncelet In papers I think we would be forced to abbreviate most of the names in any event?