Skip to content

Conversation

@jokasimr
Copy link
Contributor

@jokasimr jokasimr commented Jul 23, 2025

I was looking at addressing #176 by reducing the runtime of the beam center finder.

As part of that I noticed that the main reason the center of mass beam center finder performs relatively poorly compared to the minimization based approach is the asymmetry of the sample holder mask around the center of mass.

I fixed that by using a symmetrical mask to mask the sample holder.
In the end that method became quite simple, about as fast as the original center of mass approach, and it performs better than the minimization based approach in the sense that it achieves lower cost than the minimization procedure does (surprisingly).

This figure shows what the symmetrical mask looks like:
symmetrical-maks

Note that the center of mass is found iteratively and the mask updates each iteration based on the previous beam center guess.

Comparison to the minimization based approach

The two figures below show the IoQ curves from the old minimization based approach and the new symmetrical masks + center of mass approach.

old new

The curve associated with the North East quadrant (NE) is an outlier because it contains the beam holder.
I think that is just because of how this tutorial notebook is written. In a realistic setting the sample holder mask should be taken into account in the normalization factor and then the NE quadrant should not be an outlier any more.

@nvaytet
Copy link
Member

nvaytet commented Jul 24, 2025

Interesting. Do you happen to be able to also make the plot for the current center of mass method (with asymmetrical masks) for comparison?
Thanks!

data,
sample_holder_radius,
sample_holder_arm_width,
):
Copy link
Member

Choose a reason for hiding this comment

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

Could you describe a bit what the algorithm is doing (maybe in a docstring)? It's quite hard to follow, also with the variable names like c and d.

If I followed a little, it seems this assumes masks will always be wedge-like (because sample holder will be an arm stretching out from one side). I am not sure we can make that assumption in all 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.

I'll update with a description.

c is the current beam center guess

d is the distance from the current beam center guess to each pixel

m.masks.clear()
s = m.bins.sum()

for i in range(20):
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 explain where the number 20 (and 10 below) comes from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just iteration counts that I made up. You can probably reduce them by a lot but the iterations are quick anyway so it doesn't matter.

@jokasimr
Copy link
Contributor Author

jokasimr commented Jul 24, 2025

If I followed a little, it seems this assumes masks will always be wedge-like (because sample holder will be an arm stretching out from one side). I am not sure we can make that assumption in all cases?

That's right. I'm not sure either, it would be good to see some concrete cases what the sample holder mask looks like. In this case we don't have a sample holder mask, so I just made one up.

If we have an arbitrary holder mask and want to apply this method we need so be able to symmetrize the mask around the current beam center guess. It's a bit more involved but not insurmountable. Not sure what the performance would be in that case since I haven't tried.

@jokasimr
Copy link
Contributor Author

Do you happen to be able to also make the plot for the current center of mass method (with asymmetrical masks) for comparison?

old_com

@nvaytet
Copy link
Member

nvaytet commented Jul 24, 2025

That's right. I'm not sure either, it would be good to see some concrete cases what the sample holder mask looks like. In this case we don't have a sample holder mask, so I just made one up.

If we have an arbitrary holder mask and want to apply this method we need so be able to symmetrize the mask around the current beam center guess. It's a bit more involved but not insurmountable. Not sure what the performance would be in that case since I haven't tried.

I suspect experiment teams will come in with their own sample holders, and they could have any shape (maybe multiple arms or something else?). But we should ask the instrument team / IDS to find out for sure.

@jokasimr
Copy link
Contributor Author

jokasimr commented Jul 28, 2025

For reference, this is pseudocode for how I imagine we could symmetrize an arbitrary user defined sample holder mask:

def symmetric_mask(center, mask):
   '''Returns a mask that masks all the pixels that are masked in ``mask``
     and is symmetric around ``center``.

    Assumes we can compute the center point of any pixel
     and that we can compute the index of the pixel that contains a point.
   '''
    m = mask.copy()
    
    for i in range(mask.size):

        # Pixel position mirrored in center
        im = 2 * center - position(mask, i)

        # Position of mirror image converted to pixel index
        j = position_to_index(mask, im)

        if 0 <= j < m.size and mask[i]:
            m[j] = True
            
    return m

@jokasimr
Copy link
Contributor Author

jokasimr commented Aug 26, 2025

Okay I just spoke to Oliver about this and he said the shadow in the image is probably not the sample holder, it is more likely the beam stop.
After thinking a bit about it that makes a lot of sense.

ESS SANS instruments will probably have a similar contraption, but since it is not part of the sample environment it's not something that users will ever change.
It might move occasionally, but overall the shape etc is going to be static.

This means creating a symmetrical mask to cover it is unlikely to be a problem.

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