Port beam center finder notebook from old docs and fix bug in center finder #72
Conversation
| "id": "35ba00f1-705e-4307-b338-893323bf3e20", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "## Method 2: computing $I(Q)$ inside 4 quadrants" |
There was a problem hiding this comment.
Isn't a lot of the code below in the Python module? Why is it repeated here? If it is to explain the methodology, that did not seem clear to me. If you intend to keep it, maybe first compute and plot using the Python module, and move the detailed explanation to a follow-up section?
src/esssans/beam_center_finder.py
Outdated
| out = (sc.sum(ref * c) / sc.sum(ref)).value | ||
| logger = get_logger('sans') | ||
| logger.info(f'Beam center finder: x={xy[0]}, y={xy[1]}, cost={out}') | ||
| if not np.isfinite(out): | ||
| raise ValueError( | ||
| iofq_values = all_q.values | ||
| out = 1.0e6 * iofq_values[np.isfinite(iofq_values)].max() | ||
| logger.info( | ||
| 'Non-finite value computed in cost. This is likely due to a division by ' | ||
| 'zero. Try restricting your Q range, or increasing the size of your Q bins ' | ||
| 'to improve statistics in the denominator.' | ||
| 'zero. The value was artificially changed to a large finite number. ' | ||
| 'If the results are not satisfactory, try restricting your Q range, or ' | ||
| 'increasing the size of your Q bins to improve statistics in the ' | ||
| 'denominator.' | ||
| ) | ||
| logger.info(f'Beam center finder: x={xy[0]}, y={xy[1]}, cost={out}') | ||
| return out |
There was a problem hiding this comment.
Is this an intentional double use of out? So this is returning something very different if it is not finite?
There was a problem hiding this comment.
If out is NaN, the optimizer will either fail or go round in circles and never finds a converged value.
So I'm returning a large cost instead of a NaN. I don't think I can modify the value anywhere else, because this function is used directly by the scipy wrapper.
Before, we used to raise as soon as the optimizer was trying to probe a region of space where there was almost no data. But actually, returning a large cost allows it to continue and look elsewhere.
Now that I think of it, it may work if I return np.inf instead of a large number. That would probably be better.
src/esssans/beam_center_finder.py
Outdated
| phi = data.transform_coords( | ||
| 'phi', graph=graph, keep_intermediate=False, keep_inputs=False | ||
| ).coords['phi'] | ||
| phi_bins = sc.linspace('phi', -pi, pi, 5, unit='rad') |
There was a problem hiding this comment.
Take care here when resolving the conflict.
Porting the old notebook from https://scipp.github.io/ess/techniques/sans/sans-beam-center-finder.html
Also fixed a bug in the beam center finder based on I(Q) where
phipositions for selecting pixels in the 4 quadrants were being computed on the raw uncalibrated data, instead of the calibrated data. This meant that while we were computing I(Q) using the correct pixel positions, when comparing 4 quadrants to get the curves to overlap, we were comparing data from quadrants not symmetrically divided around the newly found center, but around the origin[0, 0].