jfreissmann
Automatically collect and test models
This PR aims to simplify testing by automatically collecting the model classes available to the dashboard.
The tests are carried out by creating an instance of the model class, providing the default parameters from the model parameter .json files and then checking whether the last simulation has converged.
The model registry can also be used to construct the classes in the dashboard itself, potentially also for injecting classes from external locations into a running version of the dashboard.
I included all of the information in the input json files now, i.e.
- Fixed the model type references (some were broken)
- Integrated what was former in the variables.py -> hp_models dictionary into the json files
- Made the parameters.py (apparently) obsolete. I think we should discuss, how to access the models in this context (see README).
- Made most tests run again, some models seem to be missing configuration parameters in the input json files
The reason for not including the dashboard parameters into the params.json was simply to separate the model input data, that can and should be used outside of the dashboard, from the stuff that is needed in the latter. I agree with you, that the way it is handled now is not perfect either and should be improved.
Thank you for finding the erroneous "type" references. Some were definitely wrong, but a great chunk were including the economizer type, which was a design decision, as the "type" field originally was not ment to be a direct reference to its class, but rather a fully defined topology that could be used in e.g. filenames.
I really like the idea of the model_registry and the way you were able to reduce the test code. I think, we will have to look into how users, who only want to use the models without the dashboard, can access the former easily and intuitively, which in my opionion felt better the way it was before using get_params and importing the model class. Maybe there is a good compromise between those.
think, we will have to look into how users, who only want to use the models without the dashboard, can access the former easily and intuitively, which in my opionion felt better the way it was before using
get_paramsand importing the model class. Maybe there is a good compromise between those.
This still works in principle :). I renamed the heat pump models to include the open or closed to ...Open and ...Closed (it could be anything really). This does also eliminate the necessity to pass the econ type to the get_param method. It is encoded in the name of the model, which again knows its parameter settings including the econ_type from the input .json files.
Actually, the HeatPumpEconIHX could be also changed to handle the econ_type inside the params dictionary so there is no need for an extra keyword. And, finally the user also does not have to actually import the class. With the model_registry, we can also create a method, that just returns the instance for a given parameter setting.
params = get_params('HeatPumpEconIHXClosed')
params['ihx']['dT_sh'] = 7.5 # superheating by internal heat exchanger
hp = HeatPumpEconIHX(params=params, econ_type=params["setup"]["econ_type"])
And, finally the user also does not have to actually import the class. With the
model_registry, we can also create a method, that just returns the instance for a given parameter setting.
from heatpumps.parameters import get_params, get_model
params = get_params('HeatPumpEconIHXClosed')
params['ihx']['dT_sh'] = 7.5 # superheating by internal heat exchanger
hp = get_model(params)
print(hp)
One more side note: right now all json files are loaded on startup, this takes a bit of time. I suggest, that the data can be loaded on demand and added to the hp_models dictionary when required. This would minimize the i/o operations.
from heatpumps.parameters import get_params, get_model params = get_params('HeatPumpEconIHXClosed') params['ihx']['dT_sh'] = 7.5 # superheating by internal heat exchanger hp = get_model(params) print(hp)
I think, this looks good to me. Maybe the method should be called get_model_by_params or something like that.
One more side note: right now all json files are loaded on startup, this takes a bit of time. I suggest, that the data can be loaded on demand and added to the
hp_modelsdictionary when required. This would minimize the i/o operations.
The idea with that decision was, that for the enduser the initial start up time is less important than the responsiveness when switching between models. For the programmatic interface I agree, that only necessary parameters should be loaded in.
On another note:
As you most likely inserted the dashboard info programmatically into the input parameters, there was an issue with UTF-8 encoding, so umlauts are not correctly inserted. See for example this file:
src/heatpumps/models/input/params_hp_cascade_econ_closed_ihx.json
Excerpt
"refrig2": "R717",
"base_topology": "Kaskadierter Kreis",
"display_name": "Geschlossen | Reihenschaltung | interne W\u00dcT (Variante B)",
"nr_ihx": 2,
"econ_type": "closed",
"comp_var": "series",
"nr_refrigs": 2,
"process_type": "subcritical"
Furthermore, having the dashboard text inside the file could be problematic with the multi language support.
That thing could easily be reverted. It was just an idea to have things in one place.
One more side note: right now all json files are loaded on startup, this takes a bit of time. I suggest, that the data can be loaded on demand and added to the
hp_modelsdictionary when required. This would minimize the i/o operations.The idea with that decision was, that for the enduser the initial start up time is less important than the responsiveness when switching between models. For the programmatic interface I agree, that only necessary parameters should be loaded in.
Actually, the respective params json was loaded on demand every time when selecting the working fluid earlier. Now everything is loaded on startup, that is why it takes a little bit more. I suggest a function that is tied to the dashboard startup to load all the params, and one to load on demand (e.g. when importing a model separately as the example in the docs show).
from heatpumps.parameters import get_params, get_model params = get_params('HeatPumpEconIHXClosed') params['ihx']['dT_sh'] = 7.5 # superheating by internal heat exchanger hp = get_model(params) print(hp)I think, this looks good to me. Maybe the method should be called
get_model_by_paramsor something like that.
Sure! You can also go ahead and push on my branch :)