Implementing new monitoring feature #30
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edan-bainglass
force-pushed
the
monitoring
branch
2 times, most recently
from
df53d36
to
f28b442
Compare
ramirezfranciscof
requested changes
Aug 29, 2023
aiida_aurora/utils/analyzers.py
Outdated
| return None | ||
|
|
||
| self.current_capacity = self.capacities[-1] | ||
| self.threshold = self.threshold * self.capacities[0] |
There was a problem hiding this comment.
Does the CapacityAnalyzer get instantiated every time it has to perform the analysis or the instance gets re-used? Because if CapacityAnalyzer.analyze gets ran more than once per object it will start accumulating products with self.capacities[0], right? I would say even if now we just re-create it each time, it would be better to implement this differently to avoid side-effects.
edan-bainglass
force-pushed
the
monitoring
branch
from
f28b442
to
3abb3c0
Compare
edan-bainglass
force-pushed
the
monitoring
branch
from
3abb3c0
to
ce8ff4c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One thing I'm concerned about is if I've migrated ALL features of the monitor calcjob. In particular, did the previous implementation capture job termination due to error/cancellation? Because I don't believe this is implemented here.
Update
Recent tests raised exit code 502 (cancelled but retrieved) due to a bad battery. This confirms that the above concern HAS BEEN IMPLEMENTED in the new feature. Note that this is not implemented per monitor, but rather appears to be a global feature. Confirming with Sabastiaan...
Okay, so it's an internal feature that existed in AiiDA in general. 501 and 502 are set by us, but AiiDA's internal mechanism reacts to them. Or something like that. Still looking into it.