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

Jump to bottom

scanning probe dataclasses and back scan functionality #97

Open
wants to merge 91 commits into
base: main
Choose a base branch
from
Open

scanning probe dataclasses and back scan functionality #97

wants to merge 91 commits into from

Conversation

qku
Copy link
Contributor

@qku qku commented Sep 8, 2023 •
edited

Description

Interface

  • add new ScanSettings dataclass to hold all settings of a scanning probe measurement (closes [New Feature] scanning probe toolchain: ScanSettings #90)
  • decorate ScannerChannel, ScannerAxis, ScanConstraints and ScanData with dataclass, replace type checks by type annotations
  • implement new scanning probe dataclass hierarchy: ScanConstraints holds ScannerAxis and ScannerChannel instances, ScanData holds ScanSettings instance, ScanData does not need to hold complete ScanConstraints
  • add checker and clipping methods to ScanConstraints
  • introduce scan_settings attribute to ScanningProbeInterface
  • introduce back scan data and back scan settings getter and setter methods (closes Implement "backscan" functionality with ni interfuse. #63)
  • extend constraints with respect to configurable backward scans: add new enum BackScanCapability

Logic and GUI

  • adapt scanning probe dummy and NI scanning probe interfuse to interface changes
  • remove bool indicators (closes Scanning probe interface still uses boolean indicators for failures etc. #64)
  • refactor scanning probe logic, optimize logic and related GUI modules in preparation for configurable back scans
  • improve scanning probe dummy: new image generator class, more realistic forward and backward scan simulation
  • add widgets to configure back scan resolution and frequency from scanning GUI.
  • add config option to save backward scan data to scanning data logic.
  • improve handling of errors during start of a scan

Motivation and Context

This refactoring was initially motivated by an extension of the scanning probe measurement toolchain to scans which are not confocal/spacial. It was decided later that this is not a good design choice. However, introducing a ScanSettings dataclass allowed to add backward scanning functionality (see #63) and enabled a major refactoring of the entire toolchain towards improved code readability.

How Has This Been Tested?

This was tested with the dummy as well as nidaq hardware. There are unit tests for parts of the dataclasses.

Types of changes

  • Bug fix
  • New feature
  • Breaking change (Causes existing functionality to not work as expected)

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.
  • All changed Jupyter notebooks have been stripped of their output cells.

qku and others added 28 commits September 6, 2023 09:44
@qku
start with implementing dataclasses
c844fd3
@qku
compatibility with ScanConstraints as dataclass
4a0435c
@qku
use built-in __dict__
cbd29c2
@qku
handle dtype in ScannerChannel as str
5eb90d8
@qku
rearrange classes
c4379ec
@qku
make ScanData an dataclass
dd11160
@qku
add new ScanSettings data class
0d3660a
@qku
simplify ScanConstraints
8ca7e90
@qku
use ScalarConstraint in ScannerAxis
c6a6ef2
@qku
experimental usage of yaml_object decorator
c8acb9c
@qku
cleanup and documentation
71d3f43
@qku
move ScanConstraints down in file
8625281
@qku
rearrage file again: interface needs to be last for type annotations
0ffd105
@qku
add checkers to ScanConstraints
8d699c4
@qku
add doc and type annotations to interface
699a183
@qku
change scan_data, constraints to properties and add scan_settings
04177b2
@qku
add checkers to ScanConstraints and minor changes to interface
35a8022
@qku
use ScanSettings object and implement scan_settings property
4622895
@qku
more type annotations
abbb581
@qku
implement new ScanData
e9db8aa
@qku
enforce tuple for several init arguments
d3ea1a3
@qku
prepare ScanData for dumping and recreating as dict support
a352e57
@qku
remove yaml workaround again
a2dd75e
@qku
documentation and type annotations
deafc37
@qku
add tests for scan_constraints
9561536
@qku
make ScanData for lightweight
754a7b8
@qku
add copy of ScanData again for safety
5335941
@qku
update changelog
d54128a
@qku
Copy link
Contributor Author

qku commented Sep 8, 2023

this addresses #90

@qku qku marked this pull request as draft September 11, 2023 09:37
@qku qku changed the title scanning probe dataclasses scanning probe dataclasses and back scan functionality Apr 21, 2024
@qku
Copy link
Contributor Author

qku commented Apr 22, 2024 •
edited

@timoML I am in the process of merging the tilt correction into my branch. Two thoughts:

  1. I would rather not add allow_coordinate_transform into ScannerConstraints: it is an immutable now and therefore this flag cannot be overwritten by the mixin. There is an alternative though:
@property
def supports_coordinate_transform(self):
    # before:
    # return self.get_constraints().allow_coordinate_transform
    # change to:
    return issinstance(self, CoordinateTransformMixin)
  1. Make the default of coord_transform_info in ScanData an empty dict instead of {'enabled': False}. Whether tilt correction is enabled or not can just be found by checking whether the info dict is empty or not, right? My motivation is that setting the default value of a data class property to a non-empty dict is non-trivial.

@qku
Copy link
Contributor Author

qku commented Apr 22, 2024

The dummy toolchain is ready to test @LukePiewalker @timoML ! Not sure if the tilt correction works, I tried my best in resolving all merge conflicts.

The NI interfuse does not work yet.

@qku
Copy link
Contributor Author

qku commented Apr 23, 2024

@timoML everything should be compatible with Python 3.8 now

@timoML
Copy link
Contributor

timoML commented Apr 23, 2024 •
edited

From a first look with the scanning dummy, the tilt correction works as intended. Unchanged images are due to the 2D limitation of the dummy. I'll try to have a look on real hardware asap.

Concerning your two changes:

  1. I find it a bit more natural to have this capability in the constraints. However, your simplification is probably worth dropping it from the constraints given the lower code complexity.
  2. I would rather not derive the "enabled" flag implicitly from an empty dict. Could we maybe?
def _get_coord_transform_info(self) -> Dict[str, Any]:
      rad_2_deg = lambda phi: phi / np.pi * 180
      info = {'enabled': False}
      if self.coordinate_transform_enabled:
          info.update({
              'enabled': True,
              'transform_matrix': self._coordinate_transform_matrix.matrix,
              'tilt_angle (deg)': rad_2_deg(self._calc_matr_2_tiltangle()),
              'translation': self._coordinate_transform_matrix.matrix[0:3, -1]
           })

This way:

  • hw with supports_coordinate_transform==False: empty dict
  • hw supports_coordinate_transform==True but turned off: {'enabled':False}

@timoML
Copy link
Contributor

timoML commented Apr 29, 2024

@qku

It looks like the backscan is not implemented for the ni_scanning_probe_interfuse yet.
Is this still on your todo list?

Can't instantiate abstract class "NiScanningProbeInterfuse" with abstract methods {'configure_back_scan', 'back_scan_settings'}

@qku
Copy link
Contributor Author

qku commented Apr 29, 2024

Yes, that's correct! It's the last bigger thing that needs to be done. I think I can do it this week.

@qku
Copy link
Contributor Author

qku commented May 6, 2024

Other than the issue with invokeMethod, configurable back scans work with the NI interfuse now. I also tested it with real hardware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Neverhorst Neverhorst Awaiting requested review from Neverhorst

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@qku @Neverhorst @LukePiewalker @timoML