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

Fit linear plane in pyxem.signals.BeamShift.get_linear_plane by minimising magnitude variance #1116

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

Conversation

sivborg
Copy link
Contributor

@sivborg sivborg commented Sep 17, 2024


Fit linear plane in pyxem.signals.BeamShift.get_linear_plane by minimising magnitude variance

Checklist

  • implementation steps
  • update the Changelog
  • mark as ready for review

What does this PR do? Please describe and/or link to an open issue.
In my work, there can be electromagnetic field in the sample areas where I want to fit linear planes for descan removal. However, using the existing algorithm in pyxem.signals.BeamShift.get_linear_plane by minimising the magnitudes through least squares gives poor results.

What is better for my case is assuming the sample has uniform electromagnetic field strengths and therefore uniform deflection magnitudes, then fitting the linear planes from this. This is done by minimising the deflection magnitude variance so that ther is a constant deflection magnitude after fitting. This can be used by setting constrain_magnitude to True:

import hyperspy.api as hs
import pyxem as pxm
import numpy as np

p = [0.5] * 6  # Plane parameters
x, y = np.meshgrid(np.arange(256), np.arange(256))
base_plane_x = p[0] * x   p[1] * y   p[2]
base_plane_y = p[3] * x   p[4] * y   p[5]

# Base descan plane
base_plane = np.stack((base_plane_x, base_plane_y)).T
data = base_plane.copy()

# Add electromagnetic shifts, four domains.
shifts = np.zeros_like(data)
shifts[:128, 128:] = (10, 10)
shifts[:128, :128] = (-10, -10)
shifts[128:, 128:] = (-10, 10)
shifts[128:, :128] = (10, 10)
data  = shifts

s = pxm.signals.BeamShift(data)

# Regular plane fitting gives poor results
s_lp = s.get_linear_plane()
assert not np.allclose(s_lp.data, base_plane.data, rtol=1e-7)

# By minimising magnitude variance you get the original plane back
s_lp = s.get_linear_plane(constrain_magnitude=True)
assert np.allclose(s_lp.data, base_plane.data, rtol=1e-7)

However there are a couple issues. If there are few domains, then there will be many valid planes for this fitting, some very nonsensical. Therefore, I have exposed the initial_values parameter, which you can vary to get different results. Part of the reason is that since we fit the planes together, we have six parameters and therefore the algorithm seems to get stuck in many local minima. See the tests for an example where this is used to fix things. The algorithm is also very affected by noise, so a mask is often necessary where this is severe.

I have also added a check for the mask having a datatype of booleans. It can accept other datatypes silently, where it would most likely give wildly wrong results. So I found it useful to have an explicit check.

Regardless this is a very useful feature for me that has helped with my data-analysis. So I hope we can share it with others!

@sivborg sivborg changed the title Linear plane algorithms Fit linear plane in pyxem.signals.BeamShift.get_linear_plane by minimising magnitude variance Sep 17, 2024
@coveralls
Copy link

coveralls commented Sep 17, 2024

Coverage Status

coverage: 92.847% ( 0.03%) from 92.819%
when pulling 09b5371 on sivborg:linear_plane_algorithms
into 971a7be on pyxem:main.

@coveralls
Copy link

Coverage Status

coverage: 92.847% ( 0.03%) from 92.819%
when pulling 335a39c on sivborg:linear_plane_algorithms
into 971a7be on pyxem:main.

@CSSFrancis CSSFrancis added this to the v0.20.0 milestone Sep 20, 2024
@CSSFrancis
Copy link
Member

@sivborg This PR looks good! Would you like to add something to the "Centering the Zero Beam" example or would you like me to add that? I can help with creating a test dataset is that helps.

Copy link
Member

@CSSFrancis CSSFrancis left a comment

Choose a reason for hiding this comment

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

Just a couple of basic comments.

Comment on lines 93 to 96
[d/dx, d/dy, x_0, d/dx, d/dy, y_0]
where the first three entries are for the x-shift, being in order the
step in x, step in y and the initial value at (0, 0). Similarly for the
last three entries for the y-shift. Currently only implemented for the
Copy link
Member

Choose a reason for hiding this comment

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

Can you maybe reword this? I'm not exactly sure what these parameters are/ what they do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, these parameters are a bit hard to briefly introduce, especially as we have to differentiate between an x-shift and an x-position. I will try to reword it.

mask=None,
fit_corners=None,
initial_values=None,
constrain_magnitude=False,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for constrain_magnitude to be False instead of True by default? It seems like this would work quite well in most cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

constrain_magnitude is a bit imprecise, as I reread it. To be precise, it actually minimises the magnitude variance. So I think I will rename it constrain_magnitude_variance to be more descriptive.

And I think it will not work well by default as it does not pay attention to the actual value of the magnitude, only its variance. When fitting a linear plane to a non-electromagnetic area, which is the most common use case, you actually want to minimise the magnitude itself, which it would not try to do if constrain_magnitude_variance is True. Maybe there would be a good way to minimise both the magnitude and variance at the same time, with a better cost function or some such, but it does not seem obvious for me.

@sivborg
Copy link
Contributor Author

sivborg commented Sep 27, 2024

@sivborg This PR looks good! Would you like to add something to the "Centering the Zero Beam" example or would you like me to add that? I can help with creating a test dataset is that helps.

Thank you for looking over! I can quickly expand the example, although admittedly this is a quite niche use case as it is only necessary if (a) your entire sample is electromagnetic and (b) there are several domains of equal field strength which you can fit your plane in. But I think it might be useful for people to try here and there, so I can make it more visible through the example!

@sivborg
Copy link
Contributor Author

sivborg commented Sep 27, 2024

Thank you for looking over! I can quickly expand the example, although admittedly this is a quite niche use case as it is only necessary if (a) your entire sample is electromagnetic and (b) there are several domains of equal field strength which you can fit your plane in. But I think it might be useful for people to try here and there, so I can make it more visible through the example!

@CSSFrancis I have added the new functionality to the beam centering example, although creating the example dataset is a bit ugly and should probably be encapsulated somewhere. So please add or change anything that is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants