Patch for Xopt 2.5.4 #144
Patch for Xopt 2.5.4 #144
Conversation
wenatuhs
commented
Feb 18, 2025
wenatuhs
commented
- Use Xopt's default sanity check when compose routine
- Attempt to use TuRBO's reset method to clean up fields before starting a new run
- Move variable bound check into base env class, raise error if check fails to pass
src/badger/core_subprocess.py
Outdated
| @@ -91,6 +91,11 @@ def run_routine_subprocess( | |||
| # set required arguments | |||
| try: | |||
| routine = load_run(args["routine_filename"]) | |||
| # Call the reset generator API before starting the optimization | |||
| try: | |||
There was a problem hiding this comment.
instead of using try/except, can we use an isinstance (routine.generator, BayesianGenerator) which would be more stable?
There was a problem hiding this comment.
I think for our current application/code the two approaches would perform exactly the same. If we add in more generators with state in Xopt in the future I feel we might need more general reset() API, in that case it could be easier to just write try except blocks...
I'm of course open to further discussion, it would be great if you can show me an example where the isinstance approach is better than the try catch one, then we can change the code accordingly :)
There was a problem hiding this comment.
There are no generators outside Bayesian ones that are likely to use the same turbo controller type. There are many possible ways inside calling the turbo controller that could raise an Attribute error which in turn would fail silently due to the try except block.
There was a problem hiding this comment.
we don't want things to fail silently
There was a problem hiding this comment.
I see! Thanks for the explanation/example! That makes sense. So we don't want the potential internal Attribute errors to be ignored when we call turbo_controller.reset(). Will do the isinstance logic once the reset API works.
| try: | ||
| bound = self.get_bound(name) | ||
| except Exception: | ||
| raise BadgerEnvVarError(f"Failed to get bound for {name}") |
There was a problem hiding this comment.
this puts up a warning box in the GUI correct?
There was a problem hiding this comment.
That's right -- any issues raised during an action through GUI would result in a warning box (implemented by Shamin) :)
Will add back and use it to reset generator stats once it's working
