ResponseMatrixData "type" field overwrites "type" to None. #382

Activity

1. TeresiaOlsson commented on Sep 8, 2026

   

   TeresiaOlsson

   on Sep 8, 2026

   Member

   More actions

   The dynamic validation just creates a model to validate against from the constructor of the class. It should save what it created in an attribute validation_model if needed to check what it generated.

   Should there be a field type in the data? It looked to me from the old example data which existed that there can be? If so, the field should be there otherwise the data model is not complete. The validation should then fail if you try to create a new object of the class where there is type in the data. But not if type is missing because it accepts None for it. Is that the behaviour you see?

   Strange though why this would affect the save. Maybe there is a bug in the save function which always put the type to None?
2. kparasch commented on Sep 8, 2026

   

   kparasch

   on Sep 8, 2026

   MemberAuthor

   More actions

   The type was there to find the model with which to do the validation.

   Just like in

   type: pyaml.accelerator
   machine: sr
   facility: BESSY2
   energy: 1.7185e9
   simulators:
   - type: pyaml.lattice.simulator
     lattice: bessy2_standard_user.mat
     name: design
   .
   .
   .
   

   It is not really part of the actual data.
3. TeresiaOlsson commented on Sep 8, 2026

   

   TeresiaOlsson

   on Sep 8, 2026

   Member

   More actions

   So it should be removed both from the data model and the existing files?

   When I saw it I actually quite liked that the field was there because it reminded me of MML where it in the data sometimes saves information about which function created it. So if you have a file with a random data you can look at that field and understand what type of data it is. But then the field becomes part of the data and should be mandatory so the option to have it as None should be removed.
4. kparasch commented on Sep 8, 2026

   

   kparasch

   on Sep 8, 2026

   MemberAuthor

   More actions

   I think I am outdated on how the factory works.

   It's not part of the data model, but it should still be written in the existing files.

   According to factory.py, I need to define type or class in the configuration file. So even though "type" is not part of the orbit response matrix data, it should still be there in the output file so that the factory is able to build it.

   from factory.py:

           # Legacy module/class format:
           #   type: some.module
           #   class: MyClass        (optional; otherwise PYAMLCLASS is used)
   
           # New class_path/class alias format:
           #   class_path: some.module.MyClass
           # or
           #   class: some.module.MyClass
   
5. TeresiaOlsson commented on Sep 8, 2026

   

   TeresiaOlsson

   on Sep 8, 2026

   Member

   More actions

   There are two different models now:

   • Validation model: this is the one used to validate when creating an object. This should contain only the fields that are needed for the constructor. So no type/class since that is not input required for the constructor.

   • ConfigurationSchema: this is the data model which describes what is needed for the configuration. As minimum it has the type/class field since that is required for the factory. Normally it corresponds to type/class + the fields that are in the validation model. This is also used to generate JSON schemas for the configuration.

   I looked at where in the code objects of this class is created since it doesn't happen at the point when loading the configuration. Looks like it happens here:

       @staticmethod
       def load(filename: str) -> "ResponseMatrixData":
           """Load response matrix data from a configuration file.
   
           Parameters
           ----------
           filename : str
               Path to the response matrix configuration file.
   
           Returns
           -------
           ResponseMatrixData
               Response matrix data constructed from the configuration file.
   
           Raises
           ------
           PyAMLException
               If the specified file does not exist.
           """
           path = Path(filename)
           if path.exists():
               config_dict = load(str(path.resolve()))
               return Factory.build(config_dict, ignore_external=False)
           else:
               raise PyAMLException(f"{filename}: file not found")
   

   I'm not sure if there is a reason for the factory to build this object? Can't load just create a ResponseMatrixData object directly? That would remove the need for the type/class field in the data file. It feels to me a bit strange to have field required for the factory inside the data file because how will the users know that the field is required if it's not in the data model which describes how the field content should look like?
6. kparasch commented on Sep 8, 2026

   

   kparasch

   on Sep 8, 2026

   MemberAuthor

   More actions

   To me it feels natural to have the factory build it since it is a pyaml object. Then you can also take advantage of the validation. Originally there were also other issues, e.g. the fact that the file path was relative to the original configuration file, and the factory had access to its root path.

   I don't understand why you find it strange for the "type" to be in the configuration file. This is true for all pyaml object configurations, as you describe in the ConfigureSchema.
7. TeresiaOlsson commented on Sep 8, 2026

   

   TeresiaOlsson

   on Sep 8, 2026

   Member

   More actions

   In the new structure the validation has been moved out of the factory to separate the responsibilities. The factory now only has the responsibility to create objects whereas validation is the responsibility of the schema generator and should happen before the data is passed to the factory. At the moment validation still happens during object creation because the dynamic validation is by default on but that's just because we have not fully moved to the new way yet. When that happens, the plan is to have dynamic validation = False as default for the factory and have no validation happen during the object creation. It's this issue #328 which is still work in progress.

   Ah, you think of these files as configuration files. I thought of them as data files, similar to the lattice file. I think I think of them like that since they are not loaded and expanded into the full configuration as part of loading the configuration.
8. kparasch commented on Sep 9, 2026

   

   kparasch

   on Sep 9, 2026

   MemberAuthor

   More actions

   Solved with #383
9. kparasch

   closed this as completedon Sep 9, 2026