mantidproject / mantid

Main repository for Mantid code
https://www.mantidproject.org
GNU General Public License v3.0
211 stars 123 forks source link

Instrument view not displaying correctly after ExtractSpectra #35394

Open RichardWaiteSTFC opened 1 year ago

RichardWaiteSTFC commented 1 year ago

Describe the bug ExtractSpectra scrambling the detector placement as seen in instrument viewer

To Reproduce (1) Make workspace

# import mantid algorithms, numpy and matplotlib
from mantid.simpleapi import *
import matplotlib.pyplot as plt
import numpy as np
from testhelpers import WorkspaceCreationHelper

# load empty instrument with RectangularDetector banks and create a peak table
ws = WorkspaceCreationHelper.create2DWorkspaceWithRectangularInstrument(2, 5, 11)  # nbanks, npix, nbins
AnalysisDataService.addOrReplace("ws_rect", ws)
axis = ws.getAxis(0)
axis.setUnit("TOF")
# fake peak in spectra in middle bank 1 (centered on detID=37/spec=12 and TOF=3.5)
peak_1D = 2 * np.array([0, 0, 0, 1, 4, 6, 4, 1, 0, 0, 0])
ws.setY(12, ws.readY(12) + peak_1D)
for ispec in [7, 11, 12, 13, 17, 22]:
    ws.setY(ispec, ws.readY(ispec) + peak_1D)
    ws.setE(ispec, np.sqrt(ws.readY(ispec)))

This should look like this (bank1 side-by-side view) image

(2) Use ExtractSpectra to select half a bank

detids = np.array([[25, 30, 35], [26, 31, 36], [27, 32, 37], [28, 33, 38], [29, 34, 39]])
ws_spec = ExtractSpectra(ws, DetectorList=detids.flatten())

image

(3) Contrast this with GroupDetectors

ispecs = ws.getIndicesFromDetectorIDs([int(id) for id in detids.flatten()])
ws_grp = GroupDetectors(InputWorkspace=ws, GroupingPattern=",".join([str(ispec) for ispec in ispecs]),
                        IgnoreGroupNumber=False)

This gives the correct view image

However, if you do show detector tabels it looks like the R,tth,phi are the same for both workspaces image

RichardWaiteSTFC commented 1 year ago

I think it's a problem with the spectrum index -detector mapping (which is different in the two tables) - if you plot the result of extractY integrated over TOF and rehsaped you get this image

This is actually what you'd expect because both algorithms sort by detector ID - but the results look like what you see in instrument view after ExtractSpectra (albeit transposed)

RichardWaiteSTFC commented 1 year ago

Looks like possibly a spectrum index/number confusion?

jclarkeSTFC commented 11 months ago

Hi @RichardWaiteSTFC, it looks to me like ExtractSpectra will eventually call IndexProperty::getFilteredIndexInfo(), which will order the spectra in the order of the given indices, the indices being in the same order as their corresponding detectors. In the script above, when detids.flatten() is called it'll produce [25, 30, 35, 26, ...], so calling sort on that array would fix the problem. One fix would be to sort m_workspaceIndexList somewhere in here, what do you think?

RichardWaiteSTFC commented 11 months ago

Hi @RichardWaiteSTFC, it looks to me like ExtractSpectra will eventually call IndexProperty::getFilteredIndexInfo(), which will order the spectra in the order of the given indices, the indices being in the same order as their corresponding detectors. In the script above, when detids.flatten() is called it'll produce [25, 30, 35, 26, ...], so calling sort on that array would fix the problem. One fix would be to sort m_workspaceIndexList somewhere in here, what do you think?

I think the problem is in instrument view rather than ExtractSpectra, it may be nice to have an option to sort the spectra in the order they appear in the workspace (spectrum number?) rather than order of indices provided, but his should definitely be optional with current behaviour as default. I think as long as the spectrum-number to detector ID mapping is correct (which it appears to be in the tables I show above) the instrument view should be able to plot it... It looks to me there's some logic in the instrument view that is using workspace index instead of spectrum number or vice-versa, what do you think?

github-actions[bot] commented 5 months ago

This issue has been automatically marked as stale because it has not had activity in 6 months. It will be closed in 7 days if no further activity occurs. Allowing issues to close as stale helps us filter out issues which can wait for future development time. All issues closed by stale bot act like normal issues; they can be searched for, commented on or reopened at any point. If you'd like a closed stale issue to be considered, feel free to either re-open the issue directly or contact a developer. To extend the lifetime of an issue please comment below, it helps us see that this is still affecting you and you want it fixed in the near-future. Extending the lifetime of an issue may cause the development team to prioritise it over other issues, which may be closed as stale instead.

github-actions[bot] commented 5 months ago

This issue has been closed automatically. If this still affects you please re-open this issue with a comment or contact us so we can look into resolving it.