Add support for multiple detectors in sinograms #187
Conversation
There was a problem hiding this comment.
Thanks!!!
So I have gone through reviewing here - it might be easier to do in person, but maybe it is useful to keep a log here to remember why things are being done.
The overall idea for me would be : try to make it work in the future for some other beamline I have not seen yet. So less hardwiring of ID11 config.
ImageD11/sinograms/dataset.py
Outdated
| @@ -20,6 +20,8 @@ | |||
| - indexing and reconstructing grains | |||
|
|
|||
| """ | |||
|
|
|||
| POSSIBLE_DETECTOR_NAMES = ("frelon3", "eiger") | |||
There was a problem hiding this comment.
We should just read the name from the dataset. There should be only one entry which has:
/scan/measurement/detector -> attrs[ "interpretation" ] == "image"
ImageD11/sinograms/assemble_label.py
Outdated
| # "dty", "rot", "pz", "px", "py", "shtz", "shty", "shtx") | ||
| HEADERMOTORS_NSCOPE = ("dty", "rot", "pz", "px", "py", "shtz", "shty", "shtx") | ||
| HEADERMOTORS_TDXRD = ("diffty", "diffrz", "samtx", "samty", "samtz", "diffry", "samrx", "samry") | ||
| HEADERMOTORS = HEADERMOTORS_NSCOPE + HEADERMOTORS_TDXRD |
There was a problem hiding this comment.
The intention of the single list was to take all potentially interesting names.
ImageD11/sinograms/dataset.py
Outdated
| # open the masterfile | ||
| hname = self.masterfile | ||
| detectors_seen = [] | ||
| scan = "1.1" |
There was a problem hiding this comment.
You need to import the scans first. Then find the detector for each scan.
ImageD11/sinograms/dataset.py
Outdated
|
|
||
| with h5py.File( hname, 'r' ) as hin: | ||
| # go through the first scan, and see what detectors we can see | ||
| for measurement in list(hin[scan]['measurement']): |
There was a problem hiding this comment.
for item in hin[scan]['measurement']:
if "interpretation" in item.attrs and item.attrs[ "interpretation" ] == "image":
....
ImageD11/sinograms/dataset.py
Outdated
| if measurement in POSSIBLE_DETECTOR_NAMES: | ||
| detectors_seen.append(measurement) | ||
|
|
||
| if len(detectors_seen) != 1: |
There was a problem hiding this comment.
So the detector could be an argument to init to decide which one
ImageD11/sinograms/dataset.py
Outdated
| self.detector = detectors_seen[0] | ||
|
|
||
|
|
||
| def guess_motornames(self): |
There was a problem hiding this comment.
To be seen - I think this should come from nexus style attributes or user input to scale in the future
ImageD11/sinograms/dataset.py
Outdated
| using_tdxrd = True | ||
|
|
||
| if using_nscope and using_tdxrd: | ||
| raise ValueError("Found both nscope and tdxrd motors in positioners, not sure which one we were using!") |
ImageD11/sinograms/dataset.py
Outdated
| # return DataSet().load( h5name, h5group ) | ||
| # make sure detector and rotation motors are known | ||
| ds_obj = DataSet().load( h5name, h5group ) | ||
| ds_obj.guess_detector() |
There was a problem hiding this comment.
Silently modifying the dataset when loading it from disk seems a bit dodgy. I prefer to have a method like validate or repair or fix that makes clear what is happening.
ImageD11/sinograms/dataset.py
Outdated
|
|
||
| # it's better to determine the omegamotor, dtymotor and detector from the masterfile itself... | ||
| # however, that relies on realistic dataroot, analysisroot, etc values being passed to the init | ||
| # the default values for dataroot etc. will always give a broken masterfile path |
There was a problem hiding this comment.
The default was downloading your data and working in the same folder as the dataset.
ImageD11/sinograms/dataset.py
Outdated
| @@ -47,9 +51,16 @@ def __init__(self, | |||
| dset = "dataset"): | |||
| """ The things we need to know to process data """ | |||
|
|
|||
|
|
|||
| # it's better to determine the omegamotor, dtymotor and detector from the masterfile itself... | |||
There was a problem hiding this comment.
Then have:
... omegamotor = None, dtymotor=None, detector=None
if thing is None: guess_thing()
There was a problem hiding this comment.
If the info is supplied - the select the scan which have the motors.
Also handle the case of no dty motor.
Also handle the case of z scans vs y scans (etc, samry, samrx, diffry)
|
Good to discuss in person I think :) |
|
Quick thought this morning - it would be good to keep your guess_detector logic in a |
|
new request with changes suggested #189 |
guess_detector()andguess_motornames()functions tosinograms/datasets.pyThese functions read the
masterfileand determine whether we are usingfrelon3oreiger, and what ouromegamotoranddtymotorattributes should be (based on which motors are present ininstrument/positioners).They are called just before
import_scans()andimport_motors_from_master()respectively, to ensure we look for the correct data.These functions are also called in the
load()function - this is essential forsinograms/lina_segmenter.py:setup()import_scans()to handle broken softlinks (via a try/except cause to catch theKeyErrorthrown byh5py)With these additions and fixes, the
NotesGuidenotebook from the gold example will complete successfully with myfrelon3data.