Added new object types for representation of creep test on metals

[MarkusSchilling]

Fixes #254

Description

Object types were added to describe data of creep tests on metals.

Note

In accordance with sections defined in the “data schema” for the creep test that was used as underlying source for object type definitions, I tried to provide a modular representation of object…
I’m not fully certain whether the modular decomposition is the best long-term modeling choice. As an alternative, I also created a second “monolithic” design that collapses all fields into a single CreepTest object type with many sections (i.e., one object, many sections). If the simpler “single-object” approach is preferred, we can switch to that variant instead.

Reference

Issue #254

[JosePizarro3]

Hi @MarkusSchilling

Just a small update. I decided to do a few things myself before proceeding with a review here in GitHub:

• Move creep test objects to ./bam_masterdata/datamodel/creep_test.py, so they are easier to track
• Added MechanicalTest class inheriting from ExperimentalStep
• Added inheritance to MechanicalTest in CreepTest --> please note I did not fix the inheritance in any other class
• Apply formatting

I will now leave comments, but the definition is large: almost 3000 lines of code. This is not bad, but I will probably be only adding one comment which applies to multiple parts.

[JosePizarro3]

Also adding this here for reference

[JosePizarro3]

Very cool, amazing work you did here Markus 🥳 Thank you very much, this data is going to look great in openBIS! I also like the linked_... type of properties; you are the first one in the Data Store who make use of it, truly, so I am really happy about it 😄

Now, I left some comments. I was not exhaustive, so to avoid leaving you 300 comments. Rather, I wanted to combine them in some thing to change everywhere in these classes definitions. I can summarize them in:

• Don't be afraid of using inheritance. You can always start from other classes in object_types.py (e.g., ExperimentalStep for CreepTest, or Sample for TestPiece, or Instrument for all your classes about Measuring instrumentation)
• When you inherit, you also inherit all properties defined in the parent. This means you do not need to redefine them again in the child (e.g., you have now in CreepTest a creep_test_date_of_test_start property, but if you inherit from ExperimentalStep, you alreayd have a start_date which fulfills that metadata)
• Delete all ocurrences of show_in_edit_views in your PropertyTypeAssignment please. This is a problem from our side, so sorry!
• Try to simplify code and generate_code_prefix. code must only reflect inheritance, nothing else.
• Please, make sure all property_label are in English. Otherwise, it will be too big in the Web UI.
• Can you check all CONTROLLEDVOCABULARY that you have that could be replaced by a BOOLEAN? Those are the ones that are Yes/No in your schema. This is because creating controlled vocabularies like this, multiple times, is inefficient from the SQL-database point of view and something faster will be to have simply booleans here.
• Why some percentages and properties are VARCHAR instead of REAL? You can always force units (for percentage, you leave it as it is and simply mention it in the property_label as I suggested with pint).

In a next review, I will double check the properties to make sure that 1) you are not reusing some which have a different meaning (e.g., I suggested to change creep_test_specified_temperature from ATOM_MD_TARG_TEMP_IN_K to its own one, as they represent different things) and 2) you are not using properties which will be eventually deprecated (e.g., REQUEST.PROJECT or $ORDER.ADDITIONAL_INFORMATION)

I think you could also move here the vocabularies defined in #251 in another file. We can create a folder ./bam_masterdata/datamodel/creep_test/ and in there you copy-paste this file and also add another called vocabularies.py where you put the creep testing-specific vocabularies that you have in #251.

[JosePizarro3]

Hi @MarkusSchilling , I think something is off with the latest updates. It might be something get messed up with the fork and pushing changes?

Do you mind sharing the latest updated version of the creep_test.py and vocabularies.py? I will do it myself.

[JosePizarro3]

I included some comments as I checked the changes. These are all in the subfolder I created for your creep_test/.

I automated extracting these comments, so here they are for you:

[
  {
    "file": "object_types.py",
    "line": 97,
    "text": "is this necessary? only a boolean inside CREEP_TEST_LINK_LOAD_MEASURING_DATA_ACQUISITION",
    "raw": "# ? is this necessary? only a boolean inside CREEP_TEST_LINK_LOAD_MEASURING_DATA_ACQUISITION"
  },
  {
    "file": "object_types.py",
    "line": 252,
    "text": "check if this is abstract enough for EXPERIMENTAL_STEP",
    "raw": "# ? check if this is abstract enough for EXPERIMENTAL_STEP"
  },
  {
    "file": "object_types.py",
    "line": 825,
    "text": "check its relation with Chemical",
    "raw": "# ? check its relation with Chemical"
  },
  {
    "file": "object_types.py",
    "line": 869,
    "text": "check its relation with Chemical",
    "raw": "# ? check its relation with Chemical"
  },
  {
    "file": "object_types.py",
    "line": 913,
    "text": "check if we need to define Results object type",
    "raw": "# ? check if we need to define Results object type"
  },
  {
    "file": "object_types.py",
    "line": 939,
    "text": "check if we need to define Results object type",
    "raw": "# ? check if we need to define Results object type"
  },
  {
    "file": "object_types.py",
    "line": 975,
    "text": "check its relation with Sample",
    "raw": "# ? check its relation with Sample"
  },
  {
    "file": "object_types.py",
    "line": 1068,
    "text": "check its relation with Instrument",
    "raw": "# ? check its relation with Instrument"
  },
  {
    "file": "object_types.py",
    "line": 1175,
    "text": "probably this is related with Furnace metainformation",
    "raw": "# ? probably this is related with Furnace metainformation"
  },
  {
    "file": "object_types.py",
    "line": 1193,
    "text": "probably this is related with Holder metainformation",
    "raw": "# ? probably this is related with Holder metainformation"
  },
  {
    "file": "object_types.py",
    "line": 1211,
    "text": "is this Calibration metainformation?",
    "raw": "# ? is this Calibration metainformation?"
  },
  {
    "file": "object_types.py",
    "line": 1295,
    "text": "this is possibly related with Software metainformation",
    "raw": "# ? this is possibly related with Software metainformation"
  },
  {
    "file": "object_types.py",
    "line": 1345,
    "text": "why is this different from CREEP_TEST_TEST_MACHINE_LOADING_SYSTEM?",
    "raw": "# ? why is this different from CREEP_TEST_TEST_MACHINE_LOADING_SYSTEM?"
  },
  {
    "file": "object_types.py",
    "line": 1392,
    "text": "sometimes this CALIBRATION_STANDARD is a REAL, sometimes a CONTROLLEDVOCABULARY. Check this.",
    "raw": "# ? sometimes this CALIBRATION_STANDARD is a REAL, sometimes a CONTROLLEDVOCABULARY. Check this."
  },
  {
    "file": "object_types.py",
    "line": 1421,
    "text": "is this necessary?",
    "raw": "# ? is this necessary?"
  },
  {
    "file": "object_types.py",
    "line": 1422,
    "text": "is this an Instrument?",
    "raw": "# ? is this an Instrument?"
  },
  {
    "file": "object_types.py",
    "line": 1439,
    "text": "is this an Instrument?",
    "raw": "# ? is this an Instrument?"
  },
  {
    "file": "object_types.py",
    "line": 1539,
    "text": "extend this status to a CONTROLLEDVOCABULARY eventually",
    "raw": "# ? extend this status to a CONTROLLEDVOCABULARY eventually"
  },
  {
    "file": "object_types.py",
    "line": 1685,
    "text": "sometimes the METHOD is a REAL, sometimes is a CONTROLLEDVOCABULARY",
    "raw": "# ? sometimes the METHOD is a REAL, sometimes is a CONTROLLEDVOCABULARY"
  }
]

[MarkusSchilling]

Thanks for rechecking the (newest) changes in the creep test related files "object_types.py" and "vocabularies.py" as well as creating the dedicated "creep_test" folder in bam-masterdata/datamodel! Furthermore, I also appreciate the extraction of the comments from the code to get a better overview thereon.

Following these, of course, I changed the aforementioned files accordingly and want to answer to the comments line by line:

Changes referring to the object_types.py file

1. Line 97 - load-measuring DAQ necessary?
   I kept the object but changed it away from Instrument. It is currently metadata about force recording, so ObjectType seems to be more appropriate.

2. Line 252 - project abstract enough for ExperimentalStep?
   I changed the project field from free text to an object link to PROJECT.

3. Line 825 - relation with Chemical?
   Not changed to Chemical. It describes composition metadata, not a chemical substance. I did not really know how to better link this to the "chemical" object.

4. Line 869 - relation with Chemical?
   Same reasoning as above (point 3).

5. Line 913 - define Results object type?
   I considered this, but did not introduce it. There is no generic Results base class in the higher-level (bam-masterdata/datamodel/)object_types.py. As a result, the result-specific objects remain ObjectType.

6. Line 939 - define Results object type?
   Same decision (see above, point 5).

7. Line 975 - relation with Sample?
   I think, there was a link to sample already, which was kept. Due to some specific properties, a specific creep test piece was defined anyhow, because just using the provided "sample" seemed to be lacking in expression...

8. Line 1068 - relation with Instrument?
   Changed CreepTestTestMachine to TestingMachine to be more specific.

9. Line 1175 - furnace metainformation?
   Kept heating system as Instrument, since the furnace (heating system) is addressed and then described by a specification that refers to a controlled vocabulary.

10. Line 1193 - holder metainformation?
    Changed holder system to InstrumentAccessory.

11. Line 1211 - calibration metainformation?
    Kept calibration metadata locally for now, but fixed certificate types and calibration-standard/method conflicts. A full refactor to generic Calibration objects would be a larger design step and not only calibration but the machine loading system is intended to be described here. Probably, the calibration part / information can be extracted from the "MachineLoadingSystem" class definition?!

12. Line 1295 - software metainformation?
    Well yes, these information are included, but they are not the only ones...

13. Line 1345 - why different from loading system?
    The intention was to describe the sensor as a separate component (which it physically is). It is connected to the loading system. Furthermore, fixed load-sensor calibration standard, which now uses a controlled vocabulary and a context-specific property code.

14. Line 1392 - calibration standard sometimes REAL, sometimes CONTROLLEDVOCABULARY
    Fixed by context-specific property codes and consistent data types.

15. Line 1421 - is this necessary?
    It was kept. Does have a sense of overengineering, but it is a useful additional information on the load data acquisition with respect to the way of force recording that might be relevant metadata for the evaluation of the resulting measurement curves.

16. Line 1422 - load DAQ an instrument?
    Changed to ObjectType.

17. Line 1439 - laboratory conditions an instrument?
    Changed to EnvironmentalConditions.

18. Line 1539 - calibration status to controlled vocabulary eventually
    Done. Added CREEP_TEST_CALIBRATION_STATUS.

19. Line 1685 - calibration method sometimes REAL, sometimes CONTROLLEDVOCABULARY
    Fixed. Temperature sensor uses a controlled vocabulary; temperature DAQ method is now VARCHAR.

Changes referring to the vocabularies.py file

Some comments led to changes in the vocabularies file, as well (they were necessary, obviously).
Hence, I added:

CREEP_TEST_CALIBRATION_STATUS
THERMOCOUPLE_TYPE
CREEP_TEST_NUMBER_OF_THERMOCOUPLES

with respective descriptions.

[JosePizarro3]

Hi @MarkusSchilling

Thanks a lot for the changes. I also added you as the CODEOWNER of these files.

I just wanted to say: thanks for bearing with me throughout this process. I think this is the first, very detailed model implemented in the BAM Data Store, and I'd dare to say, in openBIS. So congrats for the hard work. Now, let's focus on the parser and finish this use-case!

I will be merging this and implement the object types in openBIS right away.