`TelemetryProducer` calls state change hook methods regardless of state change
This is a breakout ticket from #460 regarding this review comment
Description
As part of #460 a TelemetryProducer class was added to allow for more fine grain control over which specific SmartSim entity instances should be producing telemetry output. To do this, entities simply instance an TelemetryProducer as an instance attr and then itself if just a class to track the state of a flag.
In order for entities to be able to react to a change in the flag output, developers can subclass the TelemetryProducer and override two hook methods: on_enable and on_disable. These methods were intended to be called when the state of the flag changed. However the current implementation calls them on each .enable()/.disable() call respectively, regardless of if the flag state change.
This implementation here is fairly trivial, but was leading to to some failures in #460 that we did not want to lump into that PR. They should be investigated on their own as it could be emblematic of a larger existing problem already existing in the SmartSim codebase.
How to Reproduce
Consider the following repl output:
>>> from smartsim.entity.entity import TelemetryProducer
>>>
>>> class MyProducer(TelemetryProducer):
... def on_enable(self):
... print("this will print even if already enabled!")
...
>>> mp = MyProducer(False)
>>> mp.enable()
this will print even if already enabled!
>>> mp.is_enabled
True
>>> mp.enable()
this will print even if already enabled!
>>> # ^^ Ideally this should not be printed
Expected Behavior
The on_{enable,disable} hooks should only be called when the _is_on flag changes states:
>>> mp = MyProducer(False)
>>> mp.enable()
this will print even if already enabled!
>>> mp.is_enabled
True
>>> mp.enable()
>>> # Note: no text to stdout!
Acceptance Criteria
- [ ] Implement the functionality as described above
- [ ] Add a test(s) to ensure
TelemetryProducerderivatives behave as expected - [ ] Investigate/fix the errors coming from the test suite