PR: simphony_metadata.yml and cuba.yml update

[tuopuu]

This PR addresses issues #1, #4, #5, #6.

[x] Move cuba.yml from simphony-common to simphony-metadata [x] Translate cuba.yml to the same format (YAML) as simphony_metadata.yml.

[tuopuu]

@ahashibon, I'll make a new push shortly, please take a quick look then. I'm translating cuba.yml to the "new format" we're using with simphony_metadata.yml.

I also have a question about visualization: We've used the abbreviation VIS to distinquish keys that are used for visualization tools. However, now we don't have visualization defined in the metadata. Is it included in one of the already defined computational models, or should we add a new one for visualization tools?

[ahashibon]

@tuopuu let us discuss this in details with the SSB and @itziakos here, this may require too many changes deep inside.

[tuopuu]

@ahashibon, OK, but I'll make the changes here for now, and for people to take a look and comment. To my understanding these changes only require a small change to the cuba_generate script which generates enumerated Python objects for CUBA keys.

In any case, it's very easy to revert back to the previous format of cuba.yml.

[ahashibon]

@tuopuu: OK, go ahead! for more detailed discussions we can revert to the above wiki page, were next steps beyond simple conversion will be discussed if needed.

[ahashibon]

I also have a question about visualization: We've used the abbreviation VIS to distinguish keys that are used for visualization tools. However, now we don't have visualization defined in the metadata. Is it included in one of the already defined computational models, or should we add a new one for visualization tools?

we should stick to models and methods according to the ROMM. CUBA.VISUALISATION as a child of CUBA.PROCESSING could be defined. I assume we are going to leave the domain attribute as is?

[tuopuu]

I think it makes sense to remove domain from basic CUBA attributes. Most of them are already used universally in all models. Especially in that case, the domain attribute is irrelevant. If there is a need to restrict the use of some of the keys, then we could consider moving them to simphony_metadata.yml instead, so that cuba.yml would only have "universal" attributes in it.

There are some open questions for people to comment so, please, see the wiki page @ahashibon linked above.

[kitchoi]

As you are editing the yaml file, would you also correct these as well please? I ran into them when I was attempting some parser codes.
https://github.com/simphony/simphony-metadata/pull/8/files#diff-ee11a1e13dd98ffdcd89c6b550a3bc71L199 (Note: this WIP PR for code generator is not meant for being merged but for discussion or reference etc)

[tuopuu]

As you are editing the yaml file, would you also correct these as well please? I ran into them when I was attempting some parser codes.

Thanks, Kit, I had those fixed in this branch.

[kitchoi]

Thanks @tuopuu !

[kitchoi]

These CUBA keys are defined in the simphony_metadata.yml but not in cuba.yml:

['ATOMISTIC', 'CHARGE', 'CHARGE_DENSITY', 'COMPRESSIBILITY_MODEL', 'COMPUTATIONAL_MODEL', 'CONDITION', 'CONSTANT_ELECTROSTATIC_FIELD_MODEL', 'CONTINUUM', 'CUDS_COMPONENT', 'CUDS_ITEM', 'DATA', 'DESCRIPTION', 'ELECRON_MASS', 'ELECROSTATIC_FIELD', 'ELECTRIC_FIELD', 'ELECTRONIC', 'ELECTROSTATIC_FIELD', 'ELECTROSTATIC_MODEL', 'ENERGY', 'GRAVITY_MODEL', 'HEAT_CONDUCTIVITY', 'INCOMPRESSIBLE_MODEL', 'INITIAL_VISCOSITY', 'INTERATOMIC_POTENTIAL', 'ISOTHERMAL_MODEL', 'LAMINAR_FLOW_MODEL', 'LINEAR_CONSTANT', 'MATERIAL', 'MATERIAL_RELATION', 'MAXIMUM_VISCOSITY', 'MESCOCOPIC', 'MESOSCOPIC', 'MINIMUM_VISCOSITY', 'MODEL_EQUATION', 'MOMENTUM', 'MOMENT_INERTIA', 'MULTIPHASE_MODEL', 'NEWTONIAN_FLUID_MODEL', 'NEWTONIAN_MODEL', 'PAIR_POTENTIAL', 'PHYSICS_EQUATION', 'PHYSICS_VARIABLES', 'POTENTIAL_ENERGY', 'POWER_LAW_INDEX', 'RELAXATION_TIME', 'RHEOLOGY_MODEL', 'SINGLE_PHASE_MODEL', 'SURFACE_TENSION', 'SYSTEM', 'THERMAL_MODEL', 'TIME', 'TURBULENCE_MODEL', 'USER', 'VISCOSITY']

Bolded values are typos in the yaml file.

I found these keys using this script: https://github.com/simphony/simphony-metadata/blob/add_generator/find_missing_cuba.py

[tuopuu]

@kitchoi, as far as I understood, some keys are going to be defined in simphony_metadata, and the rest in cuba.yml. There can't be any overlapping of definitions between these two files. (It does not make sense which CUBA.xxx to use, if it's defined in both files.) There are some keys in your list that we need to add to cuba.yml, but I don't know exactly how the definitions should look. I could make a draft for which people could comment and suggest changes.

[ahashibon]

Let me do this after the merge of https://github.com/simphony/simphony-metadata/pull/7, will use your script from here.

[kitchoi]

as far as I understood, some keys are going to be defined in simphony_metadata, and the rest in cuba.yml. There can't be any overlapping of definitions between these two files.

Indeed! Thanks for pointing that out. I will be updating the py script (now) to remove the CUBA keys already defined in simphony_metadata.

[tuopuu]

Let me do this after the merge of #7, will use your script from here.

Ok, great. I'll make a final push for you to review.

[tuopuu]

@ahashibon, @kitchoi please review. Did I miss something?

[kitchoi]

Thanks @tuopuu ! I have updated my find_missing_cuba.py script and run it with the new push. Here is the list of CUBA keys that appear in the simphony_metadata but not immediately under CUDS_KEYS or CUBA_KEYS:

['CHARGE', 'CHARGE_DENSITY', 'COMPRESSIBILITY_MODEL', 'DATA', 'DESCRIPTION', 'ELECTRIC_FIELD', 'ELECTRON_MASS', 'ELECTROSTATIC_FIELD', 'ELECTROSTATIC_MODEL', 'ENERGY', 'HEAT_CONDUCTIVITY', 'INCOMPRESSIBLE_MODEL', 'INITIAL_VISCOSITY', 'LINEAR_CONSTANT', 'MAXIMUM_VISCOSITY', 'MINIMUM_VISCOSITY', 'MOMENTUM', 'MOMENT_INERTIA', 'MULTIPHASE_MODEL', 'NEWTONIAN_MODEL', 'POTENTIAL_ENERGY', 'POWER_LAW_INDEX', 'RELAXATION_TIME', 'SURFACE_TENSION', 'SYSTEM', 'TIME', 'TURBULENCE_MODEL', 'USER', 'VISCOSITY', 'variables']

Do we need CUBA.variables or just "variables" is enough? Looks like CUBA.INCOMPRESSIBLE_MODEL should be CUBA.INCOMPRESSIBLE_FLUID_MODEL

[kitchoi]

Except for the CUBA.variables and CUBA.INCOMPRESSIBLE_MODEL comments, the rest looks good to me. Like @ahashibon said, we can deal with the missing CUBA in a different PR.

[tuopuu]

Do we need CUBA.variables or just "variable" is enough?

This is a good question. Maybe CUBA.VARIABLES will be needed when a model (perhaps a user-tweaked one) is saved in a file, so that it's dependencies are easier to describe. ... However, I think 'variables' is enough for now. I assume you don't mean to use singular shape here, right? 'variables' would be a basic attribute of some classes, like, 'models', 'scope' or 'definition' are. We can change this later if there is a need.

Looks like CUBA.INCOMPRESSIBLE_MODEL should be CUBA.INCOMPRESSIBLE_FLUID_MODEL

I'll change this, a good catch.

[ahashibon]

Looks like CUBA.INCOMPRESSIBLE_MODEL should be CUBA.INCOMPRESSIBLE_FLUID_MODEL this is correct, you beat me to it !

Missing CUBA should be left to a new issue.

Except than this all is OK from my side.

[ahashibon]

Note that CUBA.VARIABLES is different from variables. the former is user defined (on the object/instance level( while the latter is fixed on the metadata (class) level.

[tuopuu]

Thanks @kitchoi, @ahashibon. I'll merge and close the relevant issues.

[kitchoi]

Ah I just noticed the change in shape in CUBA.MATERIAL shape: [1,] Are we adopting this?: https://github.com/simphony/simphony-metadata/issues/9#issuecomment-196817460

[ahashibon]

yes, we could go for https://github.com/simphony/simphony-metadata/issues/9#issuecomment-196817460, unless the one that @tuopuu used is good enough (using empty field instead of -1 as a place holder?)

[kitchoi]

Because shape: [1] would mean that the list should be exactly length = 1, instead of having minimum size = 1, but material relation takes multiple materials right? or did I understand this wrongly?

[ahashibon]

Because shape: [1] would mean that the list should be exactly length = 1, instead of having minimum size = 1, but material relation takes multiple materials right? or did I understand this wrongly?

you got it right @tuopuu, I was wondering if we need to use [1,-1] to indicate the same thing as [1,]? @kitchoi would the form used by @tuopuu be parse-able ? if yes, it is more elegant than using -1 as a place holder, though both are good. What do you think?

[tuopuu]

@kitchoi, is shape: [1,] a correct yaml syntax for an array with dimensions (1,n)? Empty brackets allows an empty list of materials in a material_relation. Will that be too general?

[tuopuu]

Empty brackets allows an empty list of materials in a material_relation. Will that be too general?

Please ignore, this question. If [1,] works, we can use it. I accidentally had left [1,] in the material_relations, and thought it was an empty field [] instead.

[ahashibon]

a material relation is complete only if it is associated with - or applied to - a material, therefore a minimum of one material was defined. However, if we use [ ] then this will work for MR, as the mere inclusion of the material attribute as a list already hints that one should be provided. But of course, we loose the information about the number.

If I understand it, the only question now is whether to use: [1,] or [1,-1] for expressing the same thing: minimum of 1 element is expected, and no maximum. Is this right?

[kitchoi]

@tuopuu, @ahashibon [1,] would be parsed as [1] In my original proposal in https://github.com/simphony/simphony-metadata/issues/9#issuecomment-196817460, "minimum of 1 element is expected, and no maximum" would be described as [[1, -1]] whilst [1, -1] would mean a 2D array where its second dimension has an arbitrary shape. So far we don't have any 2D or higher dimensional entity in the metadata, but it is likely we will need it for tensors for example.

I start to wonder that maybe i should update my github profile pic to tell myself apart @tuopuu :)

[tuopuu]

I start to wonder that maybe i should update my github profile pic to tell myself apart @tuopuu :)

Haha, yes. :D I could do that too.

To the point, using -1 is not very elegant looking, and also confusing for people trying to understand metadata structure. I propose we use an empty brackets for now, as this is the only example where it's used right now. We can agree on the final format later, and possibly apply that to material_relations if needed. Does that make sense to you?

[kitchoi]

I do agree that -1 is not elegant. Just thought of another way.

I propose we use an empty brackets for now, as this is the only example where it's used right now.

I agree. So we can merge this PR now and keep #9 open for discussion?

Thanks @tuopuu and @ahashibon!

[tuopuu]

Thanks @kitchoi. I'll merge so that Adham can start working on the missing keys.