fixes #236

[jonwright]

No description provided.

[jonwright]

@jadball : the index function which is at the end of indexing.py could be altered but I think that is a different story. The code in indexing.py is based on minpks + hkl_tol and we seem to need something else.

[jonwright]

Slight changes to sort out the index function. Also ran "black" to cleanup. So the diff is ruined.

[jadball]

Found the new function at the bottom of the diff.
It would be nice if we passed separate hkl_tols and minpks lists, and we iterated over them with two loops. At the moment, the peaks is the outer loop and hkl_tol is the inner loop.
Additionally, it would be good to automatically determine the minpks values by calculating how many peaks we'd expect from the rings we have chosen:

https://github.com/FABLE-3DXRD/ImageD11/blob/9eb7b1835e98139a39f51efdd2d6ea4cd7fc9e01/ImageD11/nbGui/nb_utils.py#L498C1-L508C17

Finally, we need a way to be able to exclude rings from the score_all_pairs function before we determine minpeaks - perhaps with an additional argument to score_all_pairs given a list of ring numbers to avoid?

[jadball]

Added comment above

[jadball]

Found the new function at the bottom of the diff. It would be nice if we passed separate hkl_tols and minpks lists, and we iterated over them with two loops. At the moment, the peaks is the outer loop and hkl_tol is the inner loop. Additionally, it would be good to automatically determine the minpks values by calculating how many peaks we'd expect from the rings we have chosen:

https://github.com/FABLE-3DXRD/ImageD11/blob/9eb7b1835e98139a39f51efdd2d6ea4cd7fc9e01/ImageD11/nbGui/nb_utils.py#L498C1-L508C17

Finally, we need a way to be able to exclude rings from the score_all_pairs function before we determine minpeaks - perhaps with an additional argument to score_all_pairs given a list of ring numbers to avoid?

I'm happy to do this tomorrow if you agree :)

[jonwright]

It would be nice if we passed separate hkl_tols and minpks

Do you have an example where a double loop (N^2) gives a better answer than zip( npks, tols )? I had the double loop before and decided to get rid of it for some reason. The order of the loops was not clear for me. With box beam I let the error increase first (grain position effect) and with weak data (small crystals) the fewer spots is better than larger errors.

We could also look at a smooth function like exp(-drlv/hkl_tol) and only count once per h,k,l,sign(y) peak. The cutoff stops being so sharp.

automatically determine the minpks values

This is issue #5. I tried to fix the omega ranges this thing because a lot of scans were not 180 degrees in the past. But we also have partial rings on detector corners, systematic absences, detector gaps and so on. This code should index those inorganic crystals without chopping the data range (minpks is easy, but completeness is a hard problem).

In practice, I usually index something and see how many peaks I am getting and then scale compared to that.

calculating how many peaks we'd expect from the rings we have chosen

exclude rings from the score_all_pairs

There is also code in sinograms/point_by_point (mixing peak selection with indexing).

I will send a pull that updates to take a list of rings. Probably we need some tests to pin down the finer points?

[jonwright]

There should be examples to run on in test/demo/eu3.gve, test/ken_simul/simAl.gve, also test/simul_1000_grains existing already. There are also generators for unknown cells for index_unknown.py

test/simul_1000_grains/idx.py uses negative cosine tol with a single pair of peaks to get all grains in one attempt.

Related : test_score_gvec_z.py is the function that was intended to replace drlv for peak position mis-match.

[jadball]

Tomorrow I will write a notebook that tests tolerance loops and a smooth function.

We should also think about writing tests/profiling from the simulated data:

• How many grains does it find that it should find?
• How many grains does it find that it shouldn't find?
• How long does it take to do that?

[jadball]

@jonwright I think we can merge this now and I'll open issues tomorrow for additions to the index function and testing it

[jonwright]

Thanks - I just remembered there is also 'grid_index_parallel' for box beam.