PC convention / Indexer vendor changes on detector.get_indexer

[Erlendos12]

Hi,
Some of our users noticed that the projection center is converted to BRUKER in the printed output when doing hough indexing, even though they passed convention="tsl" when creating the EBSD detector. The code and output looks something like this:

det = kp.detectors.EBSDDetector(
    shape=sig_shape,
    binning=binning,
    sample_tilt=s.detector.sample_tilt,
    tilt=s.detector.tilt,
    pc=pc,
    convention="tsl",
)
indexer = det.get_indexer(
    phase_list=self.phases,
    rhoMaskFrac=self.rho_mask,
    nBands=self.number_bands
)
print("Indexer Vendor", indexer.vendor)
xmap = s.hough_indexing(phase_list=self.phases, indexer=indexer, verbose=0)

Indexer Vendor: KIKUCHIPY
Hough indexing with PyEBSDIndex information:
  PyOpenCL: True
  Projection center: (0.496, 0.342, 0.519)   // This PC is BRUKER and not TSL
  Indexing 13299 pattern(s) in 26 chunk(s)
  Indexing speed: 229.76526 patterns/s

I understand that it is easier to use a single convention/vendor behind the scenes, but is it possible for the printed projection center in hough_indexing to be according to the detector convention? Some of our GUI-users found this printed PC confusing as it shows a different y-coordinate, and I just wanted to hear if there is a workaround for this (modifying the indexer maybe?).

[hakonanes]

Thank you for asking. I agree that it is undesirable to surprise users.

I have considered maintaining a user's prefered convention whenever they access PC values from an EBSDDetector. This would have to be consistent in all aspects, or it would result in more confusion... By all aspects I mean the class representation method (__repr__()), when accessing attributes (pc, pc_average, pc_flattened, pcx, pcy, pcz), when plotting values (plot(), plot_pc()), and in methods like the one you mention (EBSD.hough_indexing()). In this case, the user should basically not be able to tell that Bruker's convention is used internally. We also need to add more state, i.e. keep the given convention in an attribute (most likely private). PC arrays are usually not that large, so doing the conversion every time shouldn't take too much time and memory.

While this extra work doesn't sound like it's too much, I'm not sure it's worth it. An option is for us to inform even better that PC values are converted to Bruker's convention whenever a detector is made. When in doubt, users can convert back via methods like pc_tsl() etc, or plot the average PC to confirm the position on the detector (independent of convention). Do you think informing better in docstrings and examples is enough to ease users' minds?

[Erlendos12]

Yes, I do agree that it seems unnecessary to do all the work to make it consistent. I think it would help a lot to just put "BRUKER" in the print statement of hough indexing. Something like:

Hough indexing with PyEBSDIndex information:
  PyOpenCL: True
  Projection center (BRUKER): (0.496, 0.342, 0.519)
  Indexing 13299 pattern(s) in 26 chunk(s)
  Indexing speed: 229.76526 patterns/s

Of course we could print this information ourself before, but it would be nice if the information was next to the projection center. Please do let me know what you think.:)

[hakonanes]

Sure, I think that is fine. Feel free to open a PR with this change (but use "Bruker"). In the same PR, I'd like to inform better of the conversion to Bruker's convention upon initialization, if that's OK.

We print relevant information when doing Hough indexing, dictionary indexing and refinement. Which information we print is not consistent, but based on the input parameters and such... It might be that this should be standardized more. Something for another time, I think!

[hakonanes]

Relevant function:

kikuchipy/kikuchipy/indexing/_hough_indexing.py

Lines 378 to 397 in f463aa1

	def _get_info_message(nav_size: int, chunksize: int, indexer: "EBSDIndexer") -> str:
	from kikuchipy import _pyopencl_context_available
	info = (
	"Hough indexing with PyEBSDIndex information:\n"
	f" PyOpenCL: {_pyopencl_context_available}\n"
	" Projection center"
	)
	n_chunks = int(np.ceil(nav_size / chunksize))
	pc = indexer.PC.squeeze()
	if pc.size > 3:
	pc = pc.mean(0)
	info += " (mean)"
	pc = pc.round(4)
	info += (
	f": {tuple(pc)}\n" f" Indexing {nav_size} pattern(s) in {n_chunks} chunk(s)"
	)
	return info

I suggest to add it as "(Bruker, mean)" or "(Bruker)", depending on whether the mean PC or single PC is printed.

[Erlendos12]

Thanks! I will open a PR and let you know when the changes are ready. Is this considered a patch for 0.8.3, or should I do the changes in the dev branch?

[hakonanes]

Right, so this is not a bug, so technically it should be part of the next minor release. However, I'm not sure when that will be, and I realize that you would like this change propagated to your users fast. Therefore, we can make a patch release with this change. I suggest you make a PR (from a feature branch!) to main, I do my changes to your branch, and then we release 0.8.3.