Local loop verify #8
Conversation
| self.err = self.df.apply(lambda t: self.err_flag(t), axis=1) | ||
| self.result = pd.Series(index=self.df.index) | ||
| err_start_time = None | ||
| first_flag = True |
There was a problem hiding this comment.
first_flag does not seem to be used.
There was a problem hiding this comment.
thanks good catch
| err_start_time = None | ||
| err_time = 0 | ||
|
|
||
| if err_time > 1 and (not self.saturation.loc[cur_time]): |
There was a problem hiding this comment.
I use > 1 to make it a tiny bit more difficult to fail
| points = ["feedback_sensor", "set_point", "cmd", "cmd_max"] | ||
|
|
||
| def saturation_flag(self, t): | ||
| if 0 <= t["cmd_max"] - t["cmd"] <= 0.01: |
There was a problem hiding this comment.
Should the 0.01 be a "user-input" with a default value? Similarly to how we handle tolerances.
There was a problem hiding this comment.
Is there any circumstances where the command might be negative for some reason? In other words should we use abs(t["cmd_max"] - t["cmd"])?
There was a problem hiding this comment.
for 1), my intention is to require as little user inputs / config as possible, so that 0.01 is used. We can certainly change it to a parameter later;
for 2), if it is negative, I would prefer to fail the verification or raise an error. For now, if it is negative, then this would be considered "not saturated", so it has a larger chance of reporting fail than pass.
| return False | ||
|
|
||
| def err_flag(self, t): | ||
| if t["feedback_sensor"] > t["set_point"]: |
There was a problem hiding this comment.
Same comment as above for the command. I doubt it but still wanted to raise the question in case you think that it might be an issue.
There was a problem hiding this comment.
To me this is a different issue from the comment earlier. Because this is a direct acting controller, then saturating towards maximum is the expected behavior when the sensor reading is constantly above the set point, not the other way around.
| from checklib import RuleCheckBase | ||
|
|
||
|
|
||
| class LocalLoopSaturationDirectActingMin(RuleCheckBase): |
There was a problem hiding this comment.
Would it make sense to have only one verification that would handle min/max? The logic is essentially the same.
There was a problem hiding this comment.
similar to above, I think min and max are for sensor reading on two sides of the set points separately.
| from checklib import RuleCheckBase | ||
|
|
||
|
|
||
| class LocalLoopSaturationReverseActingMax(RuleCheckBase): |
There was a problem hiding this comment.
Same question as above for reverse/direct.
| class LocalLoopSetPointTracking(RuleCheckBase): | ||
| points = ["feedback_sensor", "set_point"] | ||
|
|
||
| def error_below_5percent(self, t): |
There was a problem hiding this comment.
Should the percentages be defined through parameters (with a default value)?
There was a problem hiding this comment.
same intension as the "0.01", we can change it to parameter later for sure.
No description provided.