GaloisInc / csaf

Control Systems Analysis Framework - a framework to minimize the effort required to evaluate, implement, and verify controller design (classical and learning enabled) with respect to the system dynamics.
BSD 3-Clause "New" or "Revised" License
11 stars 4 forks source link

Improve loading files from within the components [REPLACEMENT ISSUE] #71

Closed podhrmic closed 3 years ago

podhrmic commented 3 years ago

The original issue

Id: 71
Title: Improve loading files from within the components

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

podhrmic commented 3 years ago

In GitLab by @bauer-matthews on Oct 12, 2020, 06:48

@EthanJamesLew/ @podhrmic - Do we have an approach identified here? Time estimate?

podhrmic commented 3 years ago

In GitLab by @EthanJamesLewon Oct 12, 2020, 10:56

@podhrmic -- why is it hacky? The current approach is to load the weights during a model initialization stage, which seems appropriate. This is not global and supports multiple component instances.

podhrmic commented 3 years ago

In GitLab by @podhrmic on Oct 12, 2020, 16:38

It seems hacky to me, because we are using this arbitrary function prepend_curr_path() in the python code of the component:

  1. If we have 7 components loading files, would we re-implement such function 7 times?
  2. What if the location of the zip file changes? To me, it is more idiomatic to change the toml file, because that represents the configuration of the component, rather than the python code itself.

I would be OK if we had a generic load_into_component() function that I could import from csaf

What do you think?

podhrmic commented 3 years ago

In GitLab by @bauer-matthews on Oct 15, 2020, 07:54

@EthanJamesLew?

podhrmic commented 3 years ago

In GitLab by @EthanJamesLewon Oct 15, 2020, 08:29

@podhrmic

If we have 7 components loading files, would we re-implement such function 7 times?

No, the current approach is to put such methods in a utility file (_helper.py). The low level controllers share resources and methods this way.

What if the location of the zip file changes? To me, it is more idiomatic to change the toml file, because that represents the configuration of the component, rather than the python code itself.


The file path could definitely be added to the configuration file, but it would still be the model's responsibility to load it correctly. I suppose this could be addressed by adding a small utility library for models.


This leads to two solutions

  1. Move the file management methods to a separate file (like fileops.py) (less effort, fast)
  2. Make a small library for python model programs (more effort)
podhrmic commented 3 years ago

In GitLab by @podhrmic on Oct 15, 2020, 09:27

I am onboard with solution 1)

podhrmic commented 3 years ago

In GitLab by @podhrmic on Jul 13, 2021, 14:23

@EthanJamesLewdo you think this is still in scope?

podhrmic commented 3 years ago

In GitLab by @EthanJamesLewon Jul 14, 2021, 14:59

Not relevant for the short term. Closing for now.