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

feat: XYShift block for translation transform #92

Merged

Conversation

schackey
Copy link
Contributor

Description

Re-integration of the XYShift alignment method.

Related Issue

In the new version, the default alignment was done by 'AlignReferenceSources()', which used twirl. After discussion, Lionel suggested to reintegrate the simple XYShift algorithm as most tracking issues are translational.

Type of Change

I integrated a boolean parameter 'XYShift' in the 'AlignReferenceSources()' block. Furthermore, I expanded the 'ComputeTransform' block into two new blocks: first I duplicated the block and called it 'ComputeTransformTwirl' and then I reimplemented the XYShift block from an earlier version as the 'ComputeTransformXYShift' block. By default the parameter 'XYShift' is set to False, and so by default twirl is used for alignment, as is done in ver 3.0.0.
I have tested the algorithm on a few example dates, by visually checking the resulting stack and it shows no streaks and other features indicating that alignment would have failed. However, proper testing has not been done yet.

Checklist

  • I've read the CODE_OF_CONDUCT.md document.
  • I've read the CONTRIBUTING.md guide.
  • I've updated the code style using black.
  • I've written tests for all new methods and classes that I created.
  • I've written the docstring in numpy style for all the methods and classes that I used.

@lgrcia
Copy link
Owner

lgrcia commented May 19, 2023

Thanks a lot for these additions @schackey. I will create an alignment test based on fake sources and merge if passing.

@lgrcia lgrcia changed the title XYShift revival feat: XYShift block for translation transform May 19, 2023
@lgrcia lgrcia changed the base branch from main to 3.2.0 May 19, 2023 11:34
@lgrcia lgrcia changed the base branch from 3.2.0 to feat/separate_compute_transform May 19, 2023 11:46
@lgrcia
Copy link
Owner

lgrcia commented Jun 8, 2023

Hi @schackey. Could you think of a test function for these additions?

It could be done on two images, one with fake sources and the other with the same sources but transformed with a known transform. Then we should be able to find the known transform using both of the ComputeTransform blocks you implemented.

Can you also ensure that the AlignReferenceSources is not computing the transform anymore?

Edit: I wrote the transform test in 52f8348

lgrcia added a commit that referenced this pull request Jun 10, 2023
@lgrcia lgrcia merged commit bbd30d1 into lgrcia:feat/separate_compute_transform Jun 10, 2023
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.

2 participants