Issues
- how will AMI elegantly handle dict/list attributes?
- how will AMI elegantly handle complex types like the list of times/peaks from the hsd fex (or even values/times from hsd raw)? Maybe stick to raw hsd data in AMI for now?
Proposal
Proposal | Arguments In Favor | Argument Against |
---|---|---|
Data shall be accessible with a syntax like: det = run.Detector('xppcspad') det.datatype.attr1.attr2.method(evt) | passing in event at lowest level means Detector object can be reused for many different events | |
Unlike LCLS1, the Detector interface should be used only to access per-event data, not per-run data like calibration constants | Cleaner separation of classes | Need to implement separate interface for calibration access that also takes detector name as its principal argument |
The object returned by the Detector constructor shall only need information from the run's configure transition (using the software/version information) and shall be tab-completable in ipython so users can explore the object. | ||
The object returned by Detector will have static attributes/methods (i.e. not event dependent). | Simpler for users to explore and check for missing data | Doesn't reflect the dynamic nature of the data (e.g. raw data only rarely present) |
"datatype" above corresponds to a detector interface class (e.g. "raw", "fex", "roi1") currently in psana/psana/detector/detectors.py. attr1/attr2 are used for the various fields supported by that data type (e.g. for an roi data-type, this might be the roi data, and the "corners" of the roi for that particular event). | ||
The lowest-level "method(evt)" must return None if the data is not accessible for any reason (e.g. missing detector, missing raw data, missing calibration constants). We should create a mechanism for the user to discover the reason why the data is missing. | makes user code simpler to check for missing information | |
"evt" is the only allowed argument for "method(evt)" and is mandatory. If a method doesn't use "evt", then it is an indication that it shouldn't be part of the Detector interface. | AMI ease-of-use | |
"method(evt)" shall return a fundamental type (int, float, numpy array) or a dict/list of fundamental types (e.g. for HSD chan(evt) will return a dict with key corresponding to the channel numbers). There are also more complex data not covered by these cases: e.g. xpphsd.fex.chan[0] returns a list of (times, peaks) which is a "complex type" that would need special handling in AMI. | ||
A psana-python detector interface class name must follow the naming convention "dettype_drpalg_major_minor_micro", e.g. (cspad_raw_2_3_42) | ||
If a detector interface needs a "channel number" concept, that channel number shall be a pair consisting of (segment, channel) and shall be used with the lowest-level attributes, e.g. hsd.waveforms(evt)[segment][channel] | Allows multiple segments to have for example, channel 0. |
Implementation (Schematic)
In psana, the detector interface is constructed roughly like the following:
class Container: def __init__(self): pass class mybase(DetectorImpl): def calib(self): return mikhails_calib_works_with_both_rois_and_daq(self.raw()) class epix10k_daq_0_0_1(mybase): def __init__(self): super().__init__() class epix10k_rois_0_0_1(mybase): def __init__(self): super().__init__() det = Container() setattr(det,'daq',epix10k_daq_0_0_1()) setattr(det,'rois',epix10k_rois_0_0_1()) det.daq.calib() det.rois.calib()
Epics
Also see this page for some Detector Interface behavior: Raw Data Python Interface
For Epics variables we need two names: the complex epics variables name (e.g. HX2:DVD:GCC:01:PMON) and a simple user alias e.g. STAGE_POSITION. Mona currently allows variables in the epics-store to have illegal python names (i.e. with ":" in them). This is different than other detector names.
Proposal:
- the DAQ reads epicsArch.txt. it can use either the epics name or the alias as the detname in xtc2. This keeps things "natural" in xtc2, where each detector has one and only one user name. Silke says the alias was optional in the LCLS1 epicsArch.txt file.
- in epicsArch right now, Ric/Matt now use the epics variable name (was "value" previously) and ignores the alias. This would change to "prefer" the alias if it existed in epicsArch.txt, but fall back to the epics variable name.
- the DAQ optionally stores an extra epics_info object in the configure transition. (an analog of the LCLS1 "alias map" object). This single object would contain ancillary information about each epics variable, e.g.
- the full epics name
- the alias name (defaults to the full name if not specified in epicsArch.txt)
- anything else we decide we would like to add
- the epics_info object would have its own Detector interface
- the epics_info object (and Detector interface) could expand over time, presumably in a backwardly-compatible way by using the usual middle-version-number increment pattern. Might be easiest if new fields for each variable were added to the end of the struct
- PvaDetector could use the same epics_info mechanism (see some other thoughts on PvaDetector here).
Area Detector Brainstorming (AreaDetector)
Jan. 29, 2019
Aug. 4, 2020 (cpo, Mikhail, Valerio)
- det = run.Detector(det_name)
- det.raw.raw(evt) returns 2D array for single-segment detectors, dict for multi-segment detectors. key is panel number (integer). similarly for det.raw.calib(evt)
- have a flag if we want to return single-segment detectors as a dict as well (e.g. for uniformity in psocake)
- maybe image(evt, array=array) should be an algorithm? det.raw.image(evt,array=None)? or could split det.raw.image(evt), det.raw.image(array=array)? ideally shouldn't have to pass evt.
- get_calib_const(det_name, timestamp or run or evt). Mikhail suggests we should consider det.get_calib_const(timestamp or run or evt). cpo believes it should be separate from the detector interface (unlike LCLS1)
- flexible number of segments (no hardwired "32" segments for a cspad). e.g. a "full detector" may have segments 2,3,4,7,8,9. one run may have segments 3,4,7 another run may have segments 2,3,8,9
- calibration data must be taken and stored with all segments in the "full detector" (i.e. 2,3,4,7,8,9)
- try to use same common-mode algorithm for all detectors (stripes/banks). multiple passes needed for different directions. (e.g. Jungfrau and epix are currently same)
- default geometries for single panel detectors
- what do do about ROIs (future problem)?
- dict approach similar to det.raw.raw() multi-segment detectors could also be used for ROIs
- ROIs also need the information about the "corner" of the ROI
- software ROIs are really "feature extracted" data i.e. instead det.raw.image() it's det.fex.image()
- hardware ROIs should be handled in a manner similar to hsd
- how do we reuse the python code in the DRP which is multi-threaded C++? cpo suggests maybe we have a python multi-process DRP. another labor-intensive option is supporting multi-threaded C++. or write C++ and wrap with python (was complicated for the hsd, but perhaps just because we needed to avoid 1MHz malloc)
- DRP will have to work on one or more segments of a full detector (in units of panels). this is why it's important that the detector interface be very flexible with the panels it processes (e.g. for det.calib())
- epix/jungfrau offsets should be handled uniformly: like the Jungfrau so they can be changed without redeploying pedestals
- all python methods of an area detector should start with "_" except raw(),calib(),image() because we want to keep the ami interface simple.
Addendum on Jan. 5, 2021
- two categories (1) calibconst and (2) data access. cpo believes raw/calib/image in detector interface and calibration constants access has separate interface. Dubrovin thinks perhaps "det.raw.calibconst" (i.e. access through detector interface). cpo thinks bad_pixel_mask is part of "calibration constants" (category (2)) and would be used by the area detector interface (to avoid implementing it twice).
- calibconst = CalibConst('tmo_epix')
- Mikhail highlights this example: e.g. det.raw.common_mode_level(evt) should be part of the area detector interface (category (1)) because it depends on the event data
- Mikhail proposes:
- calibconst = det.raw.calibconst()
- Right now ami ignores methods with "_", the above proposal would require AMI to ignore special methods like "calibconst" because it doesn't follow the det.method(evt) pattern required by AMI (with "amitypes" return type). This is required by all det xface attributes.
- cpo believes that "everybody" uses AreaDetector, but few people use CalibConst. That is part of the motivation for splitting up the interfaces.
2020-09-04 Proposal for Detector interface from Mikhail
- have an option to create detector object directly, outside run
det = Detector(detname)
advantage- OO design: object detector physically exists without run
- reduce redundant dependences
- have an option to work with info from calibration db - pixel geometry, imaging of constans like pedestals, gains, masks, pixel_status.
- eliminate forks for duplication of code in
data = det.raw.raw(evt)
data = det.fex.raw(evt)
replace it with det.raw(evt)
The same is valid for det.calib() and det.image()
2020-09-08 Proposal for Geometry from Valerio
Geometry package for psana: psgeom or PSCalib
- psgeom:
- Pros:
Clean and modern interface
Clean and nice coding style
- Cons:
No single true internal geometry representation
Main mantainer not working at LCLS (!!!)
- Comments from MD:
- duplication of psana geometry code for panels and entire detector - wrong design, for conversion purpose it should inherit from psana geometry, not substitute it.
- as a result, it does not normally support hierarchal description of sub-detectors, always needs to be updated for new panel geometry which is done much earlier in psana (example epix10ka, jungfrau).
- there is no guarantee that converter works correctly (multiple issues in the past) because it uses internal engine for pixel geometry which might be different from original psana.
- Pros:
- PSCalib.GeometryAccess:
- Based on the content of this page Detector Geometry
Pros:
True internal representation of the geometry
Better integration with the rest of psana
Maintained and developed at LCLS (!!!)
Cons:
Interface is often not very Pythonic (example: vebosity bits) - MD: outdated comment - for lcls2 geometry is changed significantly
Documentation could be more accessible - MD: could be, in-line documentation is available and work in psana1, please provide documentation processor for lcls2.
- psgeom:
Proposal: I propose that we make PSCalib the default geometry package for psana
Geometry Interface improvement proposals (Work in progress):
Expose the bit flags in various functions as Python bool flags - NOTE: maybe already fixed in LCLS2. MD: it is matter of style. I like compact and clean style. In critical places of user interface like det.mask we provide methods with explicit specification of parameters.
Do not require parameters that are not strictly needed (for clarity) Example: det.daq.image(evt,array=None) could be det.daq.image(evt) or det.daq.image(array), either with two separate functions or with different behavior depending on the input data. MD: lcls2 method det.daq.image(evt,array=None) does not exist yet. In psana(1) det.image(evt,array=None) evt needs to be passed along with array.
Use more descriptive names for some parameters. Examples: oname, oindex, vbase, etc.
Make documentation more accessible (use one of the standard for docstrings?) - NOTE: maybe already fixed in LCLS2. MD: please first provide documentation processor for lcls2.
All this can be accomplished with wrappers, without breakage. MD: what is exact purpose for wrapper?
I volunteer to do that, of course
Area Detector Geometry Binning/ROI Support
May 20, 2021 (Mikhail, Valerio, Chuck, Chris)
Requirement: Chuck needs to be able to modify the IP, and Mikhail needs to be able to modify the pixel size/number (binning). Needs to be elegant for LCLS2.
- proposal 1: (geometry file does not change when binning changes)
- geometry "file" in the database represents the location of the detector and the beam center, and does not include any binning information
- Detector interface should combine the detector configuration ("seg_configs") and the geometry "file" to produce binning-dependent (or any other configuration dependent) geometry information.
- having the information in the seg_configs makes it easiest for the detector interface to access (e.g. a detector interface doesn't know about a PV)
- Could implement this with a new "MTRX:V3".
- proposal 2: (geometry file does change when binning changes)
- current implementation with MTRX:V2.
- the configuration object controls the MTRX line in the geometry
- the experimenters control the IP line in the geometry file
- advantage of (2):
- geometry conversion only needs the file, where proposal 1 needs the detector's geometry/detector interface.
- advantage of (1):
- geometry file doesn't need to be updated when binning changes, so easier for Chuck to update the IP, and easier for Mikhail since he doesn't worry about overwriting Chuck's IP position
- supports the general software ROI/binning
- Currently: some geometry information (e.g. pixel size for Epix, Jungfrau, Cspad2x1, Cspad, Epix100) is not in the geometry file but is "in the code". pnccd/rayonix/opal use MTRX which has the pixel size (supports uniform rectangular pixels). In some sense, not a big extension to have the software use the config object to compute pixels sizes (since there are hardwired pixel sizes for epix, Jungfrau).
- Other software ignores varying pixel sizes (e.g. epix border pixels)
- What do about converters? (see advantages of (1) and (2) above)
- ROIs (and software binning):
- here we talk about rectangular objects
- in general, a detector many have N roi's which could change per-event, and software binning
- keep the 1 geometry file
- currently have det.raw (full detector Detector xface).
- add det.roi which would have N roi's in it, each with an X,Y "corner coordinate" and X,Y binning, which in principle are definable per-event.
- this would use the existing geometry file (and pedestals!)
- what about pedestals for software binning? if we add pixels 1 and 2, we subtract the sum of those 2 pedestals
- the above two questions are specific examples of the general question: can we use the "full-detector" calibration constants for ROIs/software-binning?
- det.roi could need a configuration and the detector interface for it may need to access that configuration
- could have roi's that change per-event: in this case the corner-coordinate and binning would be in the l1accept data
- could have roi's that are static for the run: in this case the corner-coordinate and binning would be in the configuration data for the "roi detector".
- for more general shapes (circles, etc.) det.arbitrary_shape (have a mask). e.g. data might have x coords, y coords, intensity. detector interface would use those "raw data" attributes to return something usable by the user (e.g. padded() for hsd).
Handling Detector Interface Parameters in AMI2
- example: optionally enabling common mode
- Mikhail's initial conceptual proposal: det.calib(evt,cmpars=[1,3,'stuff'],mask=['status','edges','user'])
- more kwargs can be added in future, and they can be detector-specific
- only kwargs can be used (not args) because ami needs the standard pattern of passing an "evt" argument to every detector interface method
- the "red box" is key: it is the det.calib() method:
- if we did it on the calib() method, would want to right-click on the red box to see list of kwargs
- kwargs must be explicitly specified for AMI to automatically discover the list. it is not required to explicitly specify all kwargs, but then those are expert modes, and aren't automatically discovered by AMI. AMI could have a box to input an arbitrary kwarg (for experts)
- how would we enter the kwargs? can we do type-checking? yes, using the standard python3 type annotations. which could bring up appropriate QT widgets.
- Seshu thinks the type-annotation are required to bring up type-specific QT widget (might not be enough?). Maybe instead of type-specific QT widget, bring up a python-editor style box with a dictionary of kwargs that could be edited with standard python syntax (because it feels more general that type-specific QT widget).
- Seshu thinks that when we do the introspection to find the detector methods (e.g. calib) at that point we could also do the inspection of the kwargs so we can present them to the user to edit.
- Seshu thinks it might be doable (there are routines to get the list of kwargs and their defaults)
- kwarg types should be "standard": no defining a custom type, only numpy arrays, lists, tuple, dicts, strings, int, floats
- red box right-click would have a "select kwargs" option that would bring up the python-editor of the kwarg dictionary (with defaults)
- Seshu asks: how would the kwargs get transmitted to the workers? Workers are the only ones that see the red boxes.
- Is there a way for the kwargs to be put in the graph? Seshu thinks maybe it isn't part of the graph because the graph starts "after" the red box (the worker graph executes on data returned from a "yield"). This might be a question for Dan. The "det.calib" information must be communicated to workers somehow, but in some object other than the graph ("SourceNode in the .fc file)?
- Seshu thinks there might a zmq socket where the workers learn about the data sources (i.e. red boxes) via a PUB/SUB (presumably publish is done by the graph manager). If so (pending confirmation by Dan) the kwargs would be added to that PUB message, ideally. This is separate from the graph transmission, we think.
- can we drag in two calib boxes with different params? ideally yes. ami might not immediately support this.
- ideally we would be able to change the parameters while ami is running, since psana supports changing kwargs