Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset some turbo fields before starting new run #138

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Reset some turbo fields before starting new run #138

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e98bf6b
Reset some turbo fields before starting new run
zhe-slac Feb 1, 2025
b9b5630
Reset the BO generator fields even they are not presented
zhe-slac Feb 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/badger/gui/acr/components/routine_page.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,25 @@ def _compose_routine(self) -> Routine:
# Patch the BO generators to turn on TuRBO by default
if "turbo_controller" not in generator_params:
generator_params["turbo_controller"] = "optimize"

# Nullify a few properties in turbo that can cause issues
turbo_config = generator_params["turbo_controller"]
if type(turbo_config) is dict:
if turbo_config["name"] == "optimize":
try:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the try except statements here and below, if resetting the turbo config fails, we need it to do so in a verbose fashion

Copy link
Collaborator Author

@wenatuhs wenatuhs Feb 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are for the cases where there are no such fields, such as when the user selects and configs the BO generator manually (sometimes they only config the fields they care about), in those cases the turbo_controller read from the generator param editor would be a dict but may not contain the center_x and best_value fields. We only silently pass on those cases (except KeyError)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, but I don't think KeyError will actually be raised here if the key doesn't already exist. It would just add the field to the dict

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, nice catch, thanks! Will get the value first then!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, actually we can safely set the fields to None I guess as long as turbo_controller is a dict

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But would it matter if originally there are no such fields in the turbo_controller but we forcefully add in those fields and set them to None?

turbo_config["center_x"] = None
except KeyError:
pass
try:
turbo_config["best_value"] = None
except KeyError:
pass
elif turbo_config["name"] == "safety":
try:
turbo_config["center_x"] = None
except KeyError:
pass

env_params = load_config(self.env_box.edit.toPlainText())

# VOCS
Expand Down
Loading