Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update UB matrix if IndexPeaks called with CommonUBForAll=False and table contains only peaks from one run #38964

Open
RichardWaiteSTFC opened this issue Feb 24, 2025 · 2 comments
Assignees
Labels
Diffraction Issues and pull requests related to diffraction ISIS Team: Diffraction Issue and pull requests managed by the Diffraction subteam at ISIS Single Crystal Issues and pull requests related to single crystal
Milestone

Comments

@RichardWaiteSTFC
Copy link
Contributor

RichardWaiteSTFC commented Feb 24, 2025

Is your feature request related to a problem? Please describe.
If a peak table contains only peaks from a single run then IndexPeaks called with CommonUBForAll=False will optimise the UB but this won't be stored on the workspace

if (peaksPerRun.size() < 2) {
g_log.warning("Peaks from only one run exist but CommonUBForAll=True so peaks will be indexed with an optimised "
"UB which will not be saved in the workspace.");
}

Describe the solution you'd like

Update UB matrix if IndexPeaks called with CommonUBForAll=False and table contains only peaks from one run - in this way if RoundHKLs=False
peak.getQSampleFrame() = 2*np.pi*peaks.sample().getOrientedLattice().getUB() @ peak.getHKL()

Perhaps for backwards compatibility would require an additional parameter e.g. UpdateUB = False - but it's difficult as this only applies to some peak tables (with peaks from 1 run).

Describe alternatives you've considered
Can call CalculateUMatrix with indexed peaks and observed lattice parameters, this does seem to produce better results sometimes than e.g. FindUBUsingLatticeParameters. OptimizeCrystalPlacement also often seems to find goniometer offsets that improve the accuracy of the UB.

Additional context
How to treat tables with peaks from multiple runs? Could update the goniometer matrix for peaks from the same run such that
R_old * UB_new = R_new * UB_old?
Not sure this is intuitive/desired behaviour, though it might help with calibrating goniometer angles, axes and offsets

@RichardWaiteSTFC RichardWaiteSTFC added Diffraction Issues and pull requests related to diffraction ISIS Team: Diffraction Issue and pull requests managed by the Diffraction subteam at ISIS Single Crystal Issues and pull requests related to single crystal labels Feb 24, 2025
@RichardWaiteSTFC RichardWaiteSTFC added this to the Release 6.13 milestone Feb 24, 2025
@RichardWaiteSTFC
Copy link
Contributor Author

@zjmorgan what do you think of this? We don't have to do it, but it seems a shame to throw away a better UB!

@zjmorgan
Copy link
Contributor

Seems like a good idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Diffraction Issues and pull requests related to diffraction ISIS Team: Diffraction Issue and pull requests managed by the Diffraction subteam at ISIS Single Crystal Issues and pull requests related to single crystal
Projects
Status: Todo
Development

No branches or pull requests

3 participants