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

Why is Cpt(EpicsMotor) and EpicMotor creating different configuration_attrs? #325

Closed
ambarb opened this issue Mar 11, 2024 · 10 comments · Fixed by #335
Closed

Why is Cpt(EpicsMotor) and EpicMotor creating different configuration_attrs? #325

ambarb opened this issue Mar 11, 2024 · 10 comments · Fixed by #335
Assignees
Labels
Milestone

Comments

@ambarb
Copy link

ambarb commented Mar 11, 2024

Expected Behavior

This simple EpicsMotor
has the following descriptors:

In [61]: list(db[-1].descriptors[0]['configuration']['es_diag1_y']['data'] )
Out[61]: 
['es_diag1_y_user_offset',
 'es_diag1_y_user_offset_dir',
 'es_diag1_y_velocity',
 'es_diag1_y_acceleration',
 'es_diag1_y_motor_egu']

This makes 100% sense to me. I would not change this.

Current Behavior

But when EpicsMotor is used as a Cpt, the default descriptors (configuration data)
is incomplete (in this case, pay attention to tardis_theta, tardis_delta, and tardis_gamma hint: the last 3 items in the list)

In [3]: list(db[-1].descriptors[0]['configuration']['tardis']['data'] )
Out[3]: 
['tardis_energy',
 'tardis_energy_units',
 'tardis_energy_offset',
 'tardis_geometry_name',
 'tardis_class_name',
 'tardis_sample_name',
 'tardis_lattice',
 'tardis_lattice_reciprocal',
 'tardis_U',
 'tardis_UB',
 'tardis_reflections_details',
 'tardis_ux',
 'tardis_uy',
 'tardis_uz',
 'tardis_diffractometer_name',
 'tardis__hklpy_version',
 'tardis__pseudos',
 'tardis__reals',
 'tardis__constraints',
 'tardis__mode',
 'tardis_orientation_attrs',
 'tardis_theta_acceleration',
 'tardis_delta_user_offset',
 'tardis_gamma_velocity']

Possible Solution

I can make it match as expected by iterating through a list and setting .kind to "config" but I wondering if there is a more sustainable way to do this without having to do anything inside the custom class

@prjemian maybe you are interested. Did you do something simliar as above?

Steps to Reproduce (for bugs)

I think I provided enought links to a profile to get started

Context

I noticed this because I want to correlate some decision making in which the "user" value is reset by changed the "user_offset" PV. Others might want it to calculated scan overhead and see if a problem is develping over time, or it could be useful for flyscanning to know the velocity --- plus, this is a good for troubleshooting, expecially if these PV are easily changed in a GUI but there is no archiving of the values

Your Environment

2022-2.1-py39-tiled

@ambarb
Copy link
Author

ambarb commented Mar 11, 2024

just realized this should probably be an ophyd issue - let me know if I should move it there

@tacaswell tacaswell transferred this issue from bluesky/bluesky Mar 12, 2024
@tacaswell
Copy link
Contributor

I moved it to ophyd.

My guess is that something either in the pseudo positioner code (I looked quickly but did not see anything), your profile, or in hkl is changing the kind of the missing (sub) components.

@prjemian
Copy link
Contributor

Seems more an issue for hklpy. @ambarb Can you post a copy of the class where the Tardis and its real-space motors are defined? Something that starts like this:

class Tardis(E6C):
    theta = Component(EpicsMotor, ...

@prjemian
Copy link
Contributor

Simulating locally,

class SixCircle(hkl.SimMixin, hkl.E6C):
    """
    Our 6-circle.  Eulerian.

    Energy obtained (RO) from monochromator.
    """

    # the reciprocal axes are defined by SimMixin

    mu = Component(EpicsMotor, f"{M_MU}", kind="hinted", labels=["motor"])
    omega = Component(EpicsMotor, f"{M_OMEGA}", kind="hinted", labels=["motor"])
    chi = Component(EpicsMotor, f"{M_CHI}", kind="hinted", labels=["motor"])
    phi = Component(EpicsMotor, f"{M_PHI}", kind="hinted", labels=["motor"])
    gamma = Component(EpicsMotor, f"{M_GAMMA}", kind="hinted", labels=["motor"])
    delta = Component(EpicsMotor, f"{M_TTH}", kind="hinted", labels=["motor"])

    energy = Component(EpicsSignalRO, "BraggERdbkAO", kind="hinted", labels=["energy"])
    energy_units = Component(EpicsSignalRO, "BraggERdbkAO.EGU", kind="config")


sixc = SixCircle(IOC, name="sixc")
sixc.wait_for_connection()
sixc._update_calc_energy()

The delta motor is PV gp:m23:

delta = EpicsMotor("gp:m23", name="delta")

Compare them:

In [5]: sixc.delta.read_configuration()
Out[5]: 
OrderedDict([('sixc_delta_user_offset',
              {'value': 0.0, 'timestamp': 1710425767.554572}),
             ('sixc_delta_acceleration',
              {'value': 0.2, 'timestamp': 1710425767.554572})])

In [9]: delta.read_configuration()
Out[9]: 
OrderedDict([('delta_user_offset',
              {'value': 0.0, 'timestamp': 1710425767.554572}),
             ('delta_user_offset_dir',
              {'value': 0, 'timestamp': 1710425767.554572}),
             ('delta_velocity',
              {'value': 1.0, 'timestamp': 1710425767.554572}),
             ('delta_acceleration',
              {'value': 0.2, 'timestamp': 1710425767.554572}),
             ('delta_motor_egu',
              {'value': 'degrees', 'timestamp': 1710425767.554572})])

@prjemian
Copy link
Contributor

Let's test what this looks like in a simpler Device class:

In [12]: class Demo(Device):
    ...:     delta = Component(EpicsMotor, "gp:m23")
    ...: 

In [13]: demo = Demo("", name="demo")

In [14]: demo.delta.read_configuration()
Out[14]: 
OrderedDict([('demo_delta_user_offset',
              {'value': 0.0, 'timestamp': 1710425767.554572}),
             ('demo_delta_user_offset_dir',
              {'value': 0, 'timestamp': 1710425767.554572}),
             ('demo_delta_velocity',
              {'value': 1.0, 'timestamp': 1710425767.554572}),
             ('demo_delta_acceleration',
              {'value': 0.2, 'timestamp': 1710425767.554572}),
             ('demo_delta_motor_egu',
              {'value': 'degrees', 'timestamp': 1710425767.554572})])

@prjemian
Copy link
Contributor

Looks like some of the configuration is attenuated in the Diffractometer class. Moving this to hklpy repo...

@prjemian prjemian self-assigned this Mar 16, 2024
@prjemian prjemian added the bug label Mar 16, 2024
@prjemian prjemian transferred this issue from bluesky/ophyd Mar 16, 2024
@prjemian prjemian added this to the v1.1.1 milestone Mar 16, 2024
@prjemian
Copy link
Contributor

@ambarb Thanks for the bug report! I'll take a look. Also thanks for the link to the Tardis.

prjemian added a commit that referenced this issue May 12, 2024
prjemian added a commit that referenced this issue May 12, 2024
prjemian added a commit that referenced this issue May 12, 2024
prjemian added a commit that referenced this issue May 12, 2024
prjemian added a commit that referenced this issue May 12, 2024
prjemian added a commit that referenced this issue May 14, 2024
prjemian added a commit that referenced this issue May 14, 2024
prjemian added a commit that referenced this issue May 14, 2024
prjemian added a commit that referenced this issue May 14, 2024
prjemian added a commit that referenced this issue May 14, 2024
prjemian added a commit that referenced this issue May 14, 2024
prjemian added a commit that referenced this issue May 14, 2024
prjemian added a commit that referenced this issue May 14, 2024
@prjemian
Copy link
Contributor

@ambarb: The root cause is, as you found, due to incorrect method to set items as "config". These items are now assigned in the Diffractometer class. This should apply to all geometries. Unfortunately, orientations stored previously in databroker runs will not be changed. New orientations will be written correctly.

@ambarb
Copy link
Author

ambarb commented May 14, 2024

perfect. what version will this be so I can set some flow control to handle things correctly?

Also, I remember you exposed the version so one can figure it out in the ipython shell. for hklpy. At CSX, we have a start document that is "version" for bluesky and ophyd. Whether it is working 100% depends on an actual person updating the RE.md - so that would be a separate issue.

@prjemian
Copy link
Contributor

It will be in the version 1.1.1 release (not yet published).

Yes, the hklpy installed version string is available: hkl.__version__, as in:

In [1]: import hkl

In [2]: hkl.__version__
Out[2]: '1.1.1.dev41+g122ddb3'

APS beamlines have a startup inspired by your CSX beamline's example. In that, we set the RE.md["versions"] key:

versions = dict(
    apstools=apstools.__version__,
    bluesky=bluesky.__version__,
    databroker=databroker.__version__,
    epics=epics.__version__,
    h5py=h5py.__version__,
    intake=intake.__version__,
    matplotlib=matplotlib.__version__,
    numpy=numpy.__version__,
    ophyd=ophyd.__version__,
    pyRestTable=pyRestTable.__version__,
    python=sys.version.split()[0],
    spec2nexus=spec2nexus.__version__,
)

# Set up default metadata
RE.md["databroker_catalog"] = cat.name
RE.md["login_id"] = USERNAME + "@" + HOSTNAME
RE.md.update(iconfig.get("RUNENGINE_METADATA", {}))
RE.md["versions"] = versions  # < -------------- versions here
RE.md["pid"] = os.getpid()
RE.md["iconfig"] = iconfig

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants