Add value setter for simple logged quantities. #69
Conversation
|
Unsubscribing... @-mention or request review once it's ready for a look or needs attention. |
examples/log-mpi.py
Outdated
| class UserLogQuantity(LogQuantity): | ||
| """Logging support for arbitrary user quantities.""" | ||
|
|
||
| def __init__(self, name, value=None, unit=None, | ||
| description=None) -> None: | ||
| LogQuantity.__init__(self, name=name, unit=unit, | ||
| description=description) | ||
| self._quantity_value = value | ||
|
|
||
| def __call__(self) -> float: | ||
| """Return the actual logged quantity.""" | ||
| return self._quantity_value | ||
|
|
||
| def set_quantity_value(self, value: Any) -> None: | ||
| """Set the value of the logged quantity.""" | ||
| self._quantity_value = value |
There was a problem hiding this comment.
Would it make sense to move this to a place where it's importable?
There was a problem hiding this comment.
Sorry, somehow this fell off my radar. I am not accustomed to having logpyle PRs that I'm watching.
Yes, we could definitely move this and make it "a thing" offered by logpyle - but before we commit to doing that, I just want to point out that this is just an example of how to use the capability added by this change set - which was just the part that allows a setter function to be passed into, and then invoked from inside logging.
It would be my preference to add a class similar to this to the logging_quantities.py of MIRGE-Com, instead of a canned one offered by logpyle, but I defer to @inducer and @matthiasdiener about whether to offer this or similar as a logpyle construct.
There was a problem hiding this comment.
My vote would be to offer it here... the duplication between the two examples is already a bit much for me to bear. OTOH, if you'd like to demonstrate how (not very) hard it is to make your own LogQuantity, then maybe I could be convinced.
There was a problem hiding this comment.
I see the value in offering a base one. How about we offer a base one, and demonstrate how to make one's own one?
There was a problem hiding this comment.
(There could be a comment on the demo one that this specific thing already exists and can just be imported.)
…ogging pull. Co-authored-by: Andreas Klöckner <[email protected]>
| def set_quantity_value(self, name: str, value: Any) -> None: | ||
| """Set a the value of a named LogQuantity. | ||
|
|
||
| :param name: the name of the logged quantity. | ||
| :param value: the value of the logged quantity. | ||
| """ | ||
| if name in self.quantity_data: | ||
| value_setter = self.quantity_data[name].value_setter | ||
| if value_setter: | ||
| value_setter(value) | ||
| else: | ||
| from warnings import warn | ||
| warn(f"No value_setter defined for log quantity (name={name}).") | ||
|
|
There was a problem hiding this comment.
Make this a function instead.
| @@ -10,6 +11,26 @@ | |||
| from mpi4py import MPI | |||
|
|
|||
|
|
|||
| class PushLogQuantity(LogQuantity): | |||
There was a problem hiding this comment.
Rather than duplicating it, please move this somewhere and import from there.
| self._quantity_value = None | ||
| return val | ||
|
|
||
| def set_quantity_value(self, value: Any) -> None: |
There was a problem hiding this comment.
Naming? I think set_value would suffice.
| :param name: the name of the logged quantity. | ||
| :param value: the value of the logged quantity. | ||
| """ | ||
| if name in self.quantity_data: |
There was a problem hiding this comment.
Just access it and handle the KeyError.
| value_setter(value) | ||
| else: | ||
| from warnings import warn | ||
| warn(f"No value_setter defined for log quantity (name={name}).") |
There was a problem hiding this comment.
This should probably be an error.
|
Unsubscribing... @-mention or request review once it's ready for a look or needs attention. |
This helps illinois-ceesd/mirgecom#391, and adds a basic value-setting capability for users' logged quantities. The user would be responsible for implementing the capability user-side and making sure it does something useful. The local logpyle changes just fix it up so that such that a setter function can be registered.
fixes: #68