-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New ModelComponent
class
#671
Comments
How will this impact #432? I wonder if they are related. If so, we should consider them jointly and solve the underlying issue. |
I think this can be considered separately if the |
@DirkEilander What would be the advantage of having a class hierarchy for models? To me it sounds like a plugin developer wants to inherit from a specific component for their plugin. Are all the subclasses of Model searched for in HydroMT, or are there other reasons to inherit from the Model class? |
Some plugin specific methods are implemented at the Model rather than the Component. Also the PluginModel is in the current setup exposed the the core through entrypoints. We have a meeting on Thu afternoon to discuss this further. You're welcome to join |
Discussed in #636
Originally posted by DirkEilander November 7, 2023
Current implementation
Currently model components, such as
maps
,geoms
,grid
,mesh
, etc. are defined as properties of theModel
class or extensions such as theGridMixin
orMeshMixin
. All these properties have their own associatedread_
,write_
, andset_
methods and in some case some additional genericsetup_
methods. This is perhaps too restrictive and requires quite some duplication of code.It is restrictive in the sense that all models must use this terminology and squeeze the model data into these objects. In practice however, models may have multiple of the same components (e.g. SFINCS has a computational grid and subgrid) or model specific components are more meaningful to users (e.g. exposure and vulnerability model component classes in FIAT rather than geoms and tables).
The current approach also requires duplication of code as some components, e.g.
maps
,forcing
, andstates
are basically the same component (all dict of xarray objects) and share a lot of the code which is now partly circumvented by calling methods defined in the workflows.Discussed implementation
ModelComponent
template class with abstractread
,write
, andset
methods (and perhaps more) and a dynamicdata
property (not abstract) which reads the data when called for the first time (similar to e.g. currentGridModel.grid
property). Additonally, we should make subclasses forGrid
,Mesh
,Geoms
,Maps
,Table
, etc. The genericsetup_*
methods should also be implemented here.model_components
that can be used in theModel.read
andModel.write
classes to loop over and read/write all components. This would defy the need for the GridMixin etc.ModelComponent
(sub)class to replace methods with their own methods and add more.Model
andmodelComponent
is done usingweakref
library.GridModel.setup_grid_data_from_raster
it wouldGridModel.grid.add_data_from_raster
, etc.Model._run_log_method
Model
class and remove hidden/non clear behavior (egModel._FOLDERS
used inModel.set_root
)ModelRoot
class newModelRoot
class #728Example code implementation
The text was updated successfully, but these errors were encountered: