bluesky / hklpy

Diffractometer computation library with ophyd pseudopositioner support
https://blueskyproject.io/hklpy
BSD 3-Clause "New" or "Revised" License
4 stars 12 forks source link

support save & restore of UB matrix reflections #145

Closed prjemian closed 3 years ago

prjemian commented 3 years ago

fix #40 and fix #50

prjemian commented 3 years ago

@mrakitin @ambarb @gfabbris @jwkim-anl @strempfer -- Ready for review

prjemian commented 3 years ago

@gfabbris : Thanks for the review!

gfabbris commented 3 years ago

I looked a bit more into (in the context of making fixQ energy scans) and it seems like that adding the diffractometer to the detectors will plot all the positioners in it. This is because the diffractometer hints will just get the hints field from each component (it gets it from ophyd.Device I think):

@property
def hints(self):
    fields = []
    for _, component in self._get_components_of_kind(Kind.normal):
        c_hints = component.hints
        fields.extend(c_hints.get('fields', []))
    return {'fields': fields}

I wonder what is the best way around this issue. It looks like that modifying the diffractometer hints can fix it (see below), but maybe there is a better solution?

@property
def hints(self):
    fields = []
    for _, component in self._get_components_of_kind(Kind.hinted):
        if (~Kind.normal & Kind.hinted) & component.kind:
            c_hints = component.hints
            fields.extend(c_hints.get('fields', []))
    return {'fields': fields}
prjemian commented 3 years ago

Might be the best we can do now but I'm uncomfortable with the entire kind+hints+visualization process. Its documentation is minimal at best and very hard to locate.

Likely this fix will need to be revisited when the BestEffortCallback is revised or replaced. A note here could help for that eventuality.

ambarb commented 3 years ago

This is something I wanted to visit, but this should be in Bluesky, right? It has nothing to do with hklpy I have list of things to test, but it seems to me that bec.peaks needs to rely on index and look up the whatever you want, as a minimum change, and the entire things should be "dot-able"

prjemian commented 3 years ago

@ambarb: Agreed. The code from @gfabbris (https://github.com/bluesky/hklpy/pull/145#issuecomment-850867481) is not specific to hklpy. Wecan implement it here but it is better suited to the general discussion in ophyd but involves how bluesky's BestEffortCallback handles this information.

ambarb commented 3 years ago

@gfabbris is correct. plotting is not sustainable or useful, but LiveTable is coupled to LivePlot is coupled to bec. And not just for fixed Q E scan but for any diffractometer scan as it will be easier coding wise to put the entire diffractometer object in the detector list so you don't have to write a bunch of flow control on the data analysis side.

As for EfixQ, CSX's custom plan does not rely on having diffractometer motors in the detector list, but we did this for backwards compabiltiy if we want to use a different UB than the one at the time of collection.

prjemian commented 3 years ago

@mrakitin You weren't marked previously for review. Would you like to review?

mrakitin commented 3 years ago

Hi @prjemian, I'll review it in the second part of the week. Mon-Wed are pretty busy already.

prjemian commented 3 years ago

@mrakitin I'll make changes before the merge, thanks!