Node activity

[SouthEndMusic]

Fixes #98.

Here are some design decisions I came across:

• Basin and DiscreteControl nodes are always active
• Should terminals be active/inactive? Now yes for completeness
• Should the integral term keep integrating when a PID control node is inactive? Now yes
• Should a pump/weir under the control of a PID controller that is inactive always be off?

[visr]

• Basin and DiscreteControl nodes are always active
  • That's fine
• Should terminals be active/inactive? Now yes for completeness
  • Not sure what effect it would have? They don't do anything other than receiving flow. Would they throw and error on receiving flow?
• Should the integral term keep integrating when a PID control node is inactive? Now yes
  • Not sure, it sounds more logical if it would be set to 0 on deactivation and stay 0. But probably not very important.
• Should a pump/weir under the control of a PID controller that is inactive always be off?
  • Perhaps just retain their last value? What does it mean for a weir to be off?

[gijsber]

Basin and DiscreteControl nodes are always active

yes
Should terminals be active/inactive? Now yes for completeness
always active
Should the integral term keep integrating when a PID control node is inactive? Now yes
If the node being controlled is turned inactive, the associated (PID) control node can be inactive as well.
Should a pump/weir under the control of a PID controller that is inactive always be off?
I would think that the pump/weir has been set inactive and hence the PID controller becomes inactive. Once it activates I agree with Martijn that retaining the last value is most likely the best way

[visr]

Do I understand correctly that the 3 valid values for "active" are, true, false, and missing? Would it be easier to disallow missing and default to true?

[SouthEndMusic]

Yes, that can be handled in the parse_static function of #424.

[visr]

Shall we simply use Vector{Bool} here? It uses more memory, but should be faster to read and write, since bits don't need to be shuffled around.

[Hofer-Julian]

I was talking with Bart about this, the reason I suggested BitVector is that he was using a function that returned a BitVector, so no conversion step was necessary that way.

But if you think Vector{Bool} is faster, we should probably switch.

[visr]

Ok let's keep it the same. If it causes issues or shows up in profiling we can switch later.

Apparently the performance is not so clear cut: https://discourse.julialang.org/t/bitvector-vs-vector-bool-as-default-on-comparison-operations/49431

[visr]

Looks good to me!

Two things are still needed, some tests and not supporting missing active columns. But we can do that after #424 as you mention, or create a separate issue.

[SouthEndMusic]

Looks good to me!

Two things are still needed, some tests and not supporting missing active columns. But we can do that after #424 as you mention, or create a separate issue.

I'm not sure what precisely you mean by not supporting missing active columns, or is that now fixed?