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.

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

Fix for optimization sequence in scanning toolchain #155

Merged
merged 128 commits into from
Dec 16, 2024

Conversation

TobiasSpohn
Copy link
Contributor

Description

Fixes that the optimizer could not perform optimization sequences other than the default [2, 1], e.g. ["xy","z"]. By changing the checker function of the ConfigOption ScannerGui.optimizer_plot_dimensions to a constructor that correctly checks the constraints. Furthermore, the selection of available sequences is now handled by scanning_optimize_logic, the GUI only access it.
A signal is added to lay the ground for future implementation of adjusting the optimizer sequence during runtime.

Motivation and Context

The ScannerGui.optimizer_plot_dimensions ConfigOption is there to adjust the optimization sequence to an arbitrary combination of 2D and 1D optimizations within the constraints of the hardware (i.e. number of axes). It should be possible with e.g. 3 Axes to perform the default 2D and 1D optimization, but also any subset of these. E.g. only a 1D or multiple 1D optimizations. Due to a bug in the checker method of the ConfigOption it was not possible to select only 1D optimizations. Furthermore, it was not possible to change ScanningOptimizeLogic._scan_sequence once it was stored as a StatusVar and the default fallback for the StatusVar was hardcoded to a sequence of [2,1].

How Has This Been Tested?

Default config file

Types of changes

  • Bug fix
  • New feature
  • Breaking change (Causes existing functionality to not work as expected) -> The ConfigOption moved to the logic

Checklist:

  • My code follows the code style of this project.
  • I have documented my changes in /docs/changelog.md.
  • My change requires additional/updated documentation.
  • I have updated the documentation accordingly.
  • I have added/updated the config example for any module docstrings as necessary.
  • I have checked that the change does not contain obvious errors
    (syntax, indentation, mutable default values, etc.).
  • I have tested my changes using 'Load all modules' on the default dummy configuration. -> POI manager can't load due to [Bug] Can't load POI Manager toolchain #154
  • All changed Jupyter notebooks have been stripped of their output cells.

@TobiasSpohn TobiasSpohn requested a review from timoML September 5, 2024 15:19
@TobiasSpohn
Copy link
Contributor Author

With Hotfix #156 load all modules completes without errors.

docs/changelog.md Outdated Show resolved Hide resolved
@timoML
Copy link
Contributor

timoML commented Sep 9, 2024

That's definitely an improvement.
While testing I encountered that, currently, we get a lot of StatusVar related errors when configuring a 2-axis scanner_dummy.
Maybe we should put in some work into fixing these.
After StatusVar clearing, the behavior with 2-axis scanners is clearer now.

@TobiasSpohn
Copy link
Contributor Author

As discussed in the software meeting, all debug and Qt.BlockingQueuedConnection changes have been reverted and the branch is ready for testing with the fix for scanning. The thread deadlock issue will need some further investigation and some other PR to fix.

timoML
timoML previously approved these changes Dec 9, 2024
Copy link
Contributor

@timoML timoML left a comment

Choose a reason for hiding this comment

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

Looks good now, thank you.

Copy link
Contributor

@timoML timoML left a comment

Choose a reason for hiding this comment

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

Thank you.

@TobiasSpohn TobiasSpohn merged commit e30fd35 into main Dec 16, 2024
@TobiasSpohn TobiasSpohn deleted the 1d_scanner_optimize branch December 16, 2024 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants