-
Notifications
You must be signed in to change notification settings - Fork 93
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
Potential desync in model configuration at the model.py level #608
Comments
As a proposal, I believe we should extend our use of I think this should do the trick: @property
def name(self):
return self._model_data.attrs["name"]
@name.setter
def name(self, value: str):
self._model_data.attrs["name"] = value
@property
def inputs(self):
return self._model_data.filter_by_attrs(is_result=0)
@property
def results(self):
return self._model_data.filter_by_attrs(is_result=1)
@property
def math(self):
value = self.self._model_data.attrs["math"]
if not isinstance(value, AttrDict):
value = AttrDict(value)
return value
@math.setter
def math(self, value: dict):
if not isinstance(value, dict):
raise TypeError(
f"Attempted to add dictionary math to model, but received argument of type `{type(value).__name__}`"
)
self._model_data.attrs["math"] = AttrDict(value)
@math.deleter
def math(self):
del self._model_data.attrs["math"]
@property
def config(self):
value = self._model_data.attrs["config"]
if not isinstance(value, AttrDict):
value = AttrDict(value)
return value
@config.setter
def config(self, value: dict):
if not isinstance(value, dict):
raise TypeError(
f"Attempted to add configuration to model, but received argument of type `{type(value).__name__}`"
)
self._model_data.attrs["config"] = value
@config.deleter
def config(self):
del self._model_data.attrs["config"]
@property
def is_built(self):
return self._is_built
@property
def is_solved(self):
return self._is_solved |
4 tasks
3 tasks
irm-codebase
changed the title
Potential desync in model configuration
Potential desync in model configuration at the model.py level
Jun 25, 2024
This was referenced Jul 9, 2024
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What happened?
Calliope v0.7 keeps most of its attributes within
model._model_data.attrs[]
in the form of dictionaries.However, model math and config are replicated in
model.math
andmodel.config
. This introduces potential sync issues, necessitating some helper functions to keep these up-to-date. This is done via the_add_observed_dict()
and_add_model_data_methods()
.But this requires careful use of these functions in several parts of the code, and is a bit inefficient (we essentially duplicate the same model property).
Which operating systems have you used?
Version
v0.7
Relevant log output
No response
The text was updated successfully, but these errors were encountered: