feat: Sanitizer for adaption and flipping#909
Conversation
|
Hi @KSkwarczynski, thank you for contributing to MaCh3! Please wait for MaCh3 developers to review your PR. If no one answers within a week, please message people from this list: https://github.com/orgs/mach3-software/teams/mach3admin . While waiting, please enjoy this Use this action on your projects. Use jokes on issues instead. |
|
Hi, I am MaCh3-Telemetry bot Memory and CPU Usage SummaryMemory Usage PlotYou can view the memory usage plot directly in the job summary from the link above. |
henry-wallace-phys
left a comment
There was a problem hiding this comment.
I think the current sanitiser needs an override for "wacky" behaviours but otherwise looks great
| MACH3LOG_ERROR("You enabled adaption for parameter which has enabled flipping ({})", _fFancyNames[index]); | ||
| MACH3LOG_ERROR("Right now flipping and adapting doesn't work very well"); | ||
| MACH3LOG_ERROR("Please skip adaption for param {}, using ParametersToSkip option in config", _fFancyNames[index]); | ||
| throw MaCh3Exception(__FILE__, __LINE__); |
There was a problem hiding this comment.
So I think throwing an exception here is maybe a bit harsh, is it possible to pass it an override flag at setup?
Something like this in the YAML:
SanitizeAdaption: <True/False>
that has default behaviour of True. The default (True) will run ParamaterHandlerBase::SanitizeAdaption as is whilst False does something like
MACH3LOG_CRITICAL("You've turned of sanity checking adaption! Be incredibly careful, this will not check for correlated parameter adaptions and will allow flipped parameters to adapt without issue") ?

Pull request description
We do know adaption doesn't work very well with flipping.
So let's add sanitizer.
If this is too harsh, can change error to warning.
Changes or fixes
Examples