Deletion Candidate polar_transform_utils

[CSSFrancis]

Description
I think that we should look into profiling the polar_transform_utils with the new method for azimuthal integration to see if there is a large enough speed gain to justify having multiple methods for the same thing.

In any case this module should be rewritten to use center_direct_beam and be a function of `get_azimuthal2d rather than how it currently exists.

@din14970 any thoughts or reasons against this?

[din14970]

if performance and functionality remains the same I don't care to be honest.

The main purpose of the polar transform utility functions is to map cartesian images and templates expressed in the cartesian coordinate system to the polar domain as efficiently as possiblem so that a polar template can be slid across the polar image easily and fast for matching the in-plane Euler angle. At least: as efficiently as I could think of then; I'm convinced I could squeeze out quite a bit more performance now :).

One can then sum the transformed image over the azimuthal direction, which I think is what you mean with azimuathal integration (?), but this is not the main purpose of the utilities.

[CSSFrancis]

Just for reference the new method takes ~.3 msec to transform a 256x256 image into a polar image but around ~70 msec for a 2kx2k image. The radial method in polar_transform_utils takes around ~3msec for a 256x256 and around ~11 sec for the 2k x 2k image. The difference there is just a result of the polar_transform_utils not really sampling well in the azimuthal range....

[din14970]

could you point me to the function/method that replaces the polar_transform_utils?

[CSSFrancis]

The main purpose of the polar transform utility functions is to map cartesian images and templates expressed in the cartesian coordinate system to the polar domain as efficiently as possiblem so that a polar template can be slid across the polar image easily and fast for matching the in-plane Euler angle. At least: as efficiently as I could think of then; I'm convinced I could squeeze out quite a bit more performance now :).

One can then sum the transformed image over the azimuthal direction, which I think is what you mean with azimuathal integration (?), but this is not the main purpose of the utilities.

@din14970 Sorry azimuthal integration is a very weird way to describe these things. We are actually doing the polar unwrapping. Very similar to what you are doing here, but instead of taking the interpolation we actually sum the intensities from each cartesian pixel and then divide by the fraction of that cartesian pixel in the azimuthal pixel. The azimuthal integration comes in because we can also account for the Ewald sphere by stretching the azimuthal pixels.

We do this quickly by pre-computing things like the fraction of each cartesian pixel in the azimuthal pixel and which cartesian pixels intersect each azimuthal pixel. As far as speed goes I'm actually pretty confident in this approach. There are some histrogroamming approaches which might be slightly faster (factor of 2?), at the added cost of being less accurate. For the most part I just want to rewrite this so it can just dask-distributed/dask-cuda, reaches some degree of parity with the other method for speed and then leave any additional performance for later.

[CSSFrancis]

This is the function:

pyxem/pyxem/utils/_azimuthal_utils.py

Lines 28 to 66 in da7c8f6

	def _slice_radial_integrate(
	img,
	factors,
	factors_slice,
	slices,
	npt_rad,
	npt_azim,
	mask=None,
	): # pragma: no cover
	"""Slice the image into small chunks and multiply by the factors.
	Parameters
	----------
	img: np.array
	The image to be sliced
	factors:
	The factors to multiply the slices by
	slices:
	The slices to slice the image by
	npt_rad:
	The number of radial points
	npt_azim:
	The number of azimuthal points
	Note
	----
	This function is much faster with numba than without. There is probably a factor
	of 2-10 speedup that could be achieved by using cython or c++ instead of python
	"""
	if mask is not None:
	img = img * np.logical_not(mask)
	val = np.empty(slices.shape[0])
	for i, (s, f) in enumerate(zip(slices, factors_slice)):
	val[i] = np.sum(
	img[s[0] : s[2], s[1] : s[3]]
	* factors[f[0] : f[1]].reshape((s[2] - s[0], s[3] - s[1]))
	)
	return val.reshape((npt_azim, npt_rad)).T

It's a bit weird because it is split into two parts. The first part is precomputing a bunch of stuff:

pyxem/pyxem/utils/calibration_utils.py

Lines 250 to 266 in da7c8f6

	def get_slices2d(self, npt, npt_azim, radial_range=None):
	"""Get the slices and factors for some image that can be used to
	slice the image for 2d integration.
	Parameters
	----------
	npt: int
	The number of radial points
	npt_azim:
	The number of azimuthal points
	"""
	radial_range = self._get_radial_range(radial_range)
	# Get the slices and factors for the integration
	slices, factors, factors_slice = self._get_slices_and_factors(
	npt, npt_azim, radial_range
	)
	return slices, factors, factors_slice, radial_range

edit: (There is also a bit more of speed up when using multiple threads with Numba parallel=True)

[din14970]

It's difficult for me to figure out exactly what this is doing or how it works just from a quick read, so for me, as long as you can reproduce the calculations in the paper and it's faster then I have nothing against changing/replacing stuff.

[CSSFrancis]

It's difficult for me to figure out exactly what this is doing or how it works just from a quick read, so for me, as long as you can reproduce the calculations in the paper and it's faster then I have nothing against changing/replacing stuff.

@din14970 sounds good. Once I get the entire workflow inplace I can start to do some of the speed testing. I'm hopeful that we might be a little speed increase.

It would also be good to run the performance metrics with %env OMP_NUM_THREADS=1 as discussed here pyxem/pyxem-demos#87

[pc494]

Just noting here that I'm not planning to touch this during my sweep of the utils.

[CSSFrancis]

@pc494 This is probably good and I'd prefer removing this before the 1.0.0 release and after the 0.18.0 release