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

FindGlobalBMatrix should throw warning if no peaks indexed in a run #36863

Closed
RichardWaiteSTFC opened this issue Feb 19, 2024 · 2 comments · Fixed by #38574
Closed

FindGlobalBMatrix should throw warning if no peaks indexed in a run #36863

RichardWaiteSTFC opened this issue Feb 19, 2024 · 2 comments · Fixed by #38574
Assignees
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) 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 19, 2024

Describe the bug
FindGlobalBMatrix will not warn the user if peaks in one of the runs cannot be indexed - it just spits out a poor UB for that run - ideally if a run cannot be fitted the peaks would be excluded in the optimisation.

To Reproduce

# generate peak table
ws = LoadEmptyInstrument(InstrumentName='SXD', OutputWorkspace='ws')
axis = ws.getAxis(0)
axis.setUnit("TOF")  # need this to add peak to table

peaks = CreatePeaksWorkspace(InstrumentWorkspace='ws', NumberOfPeaks=0)
UB = np.diag([0.2, 0.25, 0.1])  # alatt = [3.8,4,10] 
SetUB(peaks, UB=UB)
SetGoniometer(Workspace='peaks', Axis0='-5,0,1,0,1')

peaks2 = CreatePeaksWorkspace(InstrumentWorkspace='ws', NumberOfPeaks=0)
UB = np.diag([0.3, 0.25, 0.1])  # alatt = [4.2,4,10] 
SetUB(peaks2, UB=UB)
SetGoniometer(Workspace='peaks', Axis0='5,0,1,0,1')

for h in range(0,3):
    for k in range(0,3):
        pk = peaks.createPeakHKL([h,k,4])
        peaks.addPeak(pk)
        pk = peaks2.createPeakHKL([h,k,4])
        peaks2.addPeak(pk)

peak_list = [peaks, peaks2]

FindGlobalBMatrix(PeakWorkspaces = peak_list,
    a=5, b=4,c=10,alpha=88,beta=88,gamma=89)
    
for ws in peak_list:
    nindexed, *_ = IndexPeaks(ws, Tolerance=0.15, CommonUBForAll=True, EnableLogging=False)
    print(f"{ws.name()} has {nindexed} indexed peaks")
    
# peaks has 9 indexed peaks
# peaks2 has 0 indexed peaks

Expected behavior
Would throw a warning and possibly not overwrite the UB on the workspace if it had one.

Screenshots

Platform/Version (please complete the following information):

  • OS: [e.g. Windows, RHEL 7, Ubuntu, macOS]
  • OS Version:
  • Mantid Version [e.g. 6.0.0]

Additional context

@RichardWaiteSTFC RichardWaiteSTFC added Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Diffraction Issues and pull requests related to diffraction ISIS Team: Diffraction Issue and pull requests managed by the Diffraction subteam at ISIS labels Feb 19, 2024
@RichardWaiteSTFC RichardWaiteSTFC added this to the Release 6.10 milestone Feb 19, 2024
@RichardWaiteSTFC RichardWaiteSTFC added the Single Crystal Issues and pull requests related to single crystal label Jun 17, 2024
@RichardWaiteSTFC
Copy link
Contributor Author

After offline discussion, there are a few things I think that can be improved:

  1. Single wrapper function around calling a child algorithm as per
    def exec_child_alg(self, alg_name: str, **kwargs):
  2. Change how initial indexing is found:
    • If no UB on any table then call FindUBUsingLatticeParameters on tables until successful then break
    • If there are UBs, copy UB to each workspace to get NUB x NUB triangular matrix of nindexed peaks - find best UB to use as a reference (leave exact method up to you - but the aim is to optimise the UB for as many runs as possible).
    • Once a reference UB has been found, index peaks in each table using this UB - if a table doesn't have enough indexed peaks then try and call FindUBUsingLatticeParameters and make_UB_consistent - if still nnot successful print warning and exclude table from optimisation.
  3. After optimisation completed, loop over workspaces, call IndexPeaks and print warning for any runs which index < MIN_NUM_INDEXED (maybe also print at debug level how many peaks are indexed in each table)
  4. Move try/except to be around the call to CalculateUMatrix in calcResiduals, so we can throw error telling user which workspace/run is problematic
  5. Relax MIN_NUM_INDEXED = 2
  6. In validateInputs require each table to have a minimum of 2 indexed peaks if a UB is present, or 6 peaks (don't need to be indexed) if no UB is present (atm just requires total of 6 peaks).

We don't need to address these in a single PR, let me know what you think/which bullet points you want to take on

Copy link

Closed by #38574.

@github-project-automation github-project-automation bot moved this from In Review to Done in ISIS Diffraction Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) 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: Done
Development

Successfully merging a pull request may close this issue.

2 participants