LiveTable failed to format row

[tangkong]

Expected Behavior

Live table shouldn't fail to format a row.

Current Behavior

LiveTable occasionally has issues formatting rows. We saw this first in a slack thread James Cryan in bp.list_scan.

Later seen by me in bp.adaptive_scan

This does not stop the scan from running, just breaks the terminal outputs. Originates from this exception catch.

Possible Solution

??? Possibly something to do with floats and ints?

Steps to Reproduce (for bugs)

In [3]: from bluesky.plans import adaptive_scan
   ...: from ophyd.sim import motor, det
   ...:
   ...: RE(adaptive_scan([det], 'det', motor,
   ...:                  start=-15,
   ...:                  stop=10,
   ...:                  min_step=0.01,
   ...:                  max_step=5.,
   ...:                  target_delta=.05,
   ...:                  backstep=True))


Transient Scan ID: 2     Time: 2022-04-19 13:33:54
Persistent Unique Scan ID: '22ecca5f-4be2-4645-ad7c-f557d3a4db61'
New stream: 'primary'
+-----------+------------+------------+------------+
|   seq_num |       time |      motor |        det |
+-----------+------------+------------+------------+
|         1 | 13:33:54.2 |        -15 |   1.39e-49 |
 failed to format row
 failed to format row
 failed to format row

RE(bp.list_scan([daq], photon_energy, list1))

(resolved by defining the list as floats, not int)

Context

Your Environment

pcds-5.3.1

[tangkong]

Pretty sure this has something to do with keeping the same type throughtout the scan. It seems LiveTable detects the data types when it receives a descriptor document, then expects those types to remain the same throughout the run. Integers can be formatted as floats, but not the other way around.

Simply put, if LiveTable sees an integer first, all following values must be integers.

For example the following succeeds since we saw a float first:

In [51]: list1 = [-1., -0.1, 0, 0.1, 1]
    ...: RE(bp.list_scan([det], motor, list1))


Transient Scan ID: 11     Time: 2022-04-21 15:35:13
Persistent Unique Scan ID: 'd0916d6a-f755-46e1-9506-20dadfd0822a'
New stream: 'primary'
+-----------+------------+------------+------------+
|   seq_num |       time |      motor |        det |
+-----------+------------+------------+------------+
|         1 | 15:35:13.6 |         -1 |      0.607 |
|         2 | 15:35:13.6 |       -0.1 |      0.995 |
|         3 | 15:35:13.7 |          0 |          1 |
|         4 | 15:35:13.7 |        0.1 |      0.995 |
|         5 | 15:35:13.7 |          1 |      0.607 |
+-----------+------------+------------+------------+
generator list_scan ['d0916d6a'] (scan num: 11)

But the this fails because the float values cannot be formatted as integers (mind the definition of list1)

In [2]: list1 = [-1, -0.1, 0, 0.1, 1]
   ...: RE(bp.list_scan([det], motor, list1))


Transient Scan ID: 1     Time: 2022-04-21 15:37:07
Persistent Unique Scan ID: '96158fdf-e5fc-439c-9d19-e893f5896e0e'
+-----------+------------+------------+------------+
|   seq_num |       time |      motor |        det |
+-----------+------------+------------+------------+
|         1 | 15:37:07.0 |         -1 |      0.607 |
 failed to format row
|         3 | 15:37:07.9 |          0 |          1 |
 failed to format row
|         5 | 15:37:07.9 |          1 |      0.607 |
+-----------+------------+------------+------------+
generator list_scan ['96158fdf'] (scan num: 1)

The adaptive scan plan can solved in a similar way if you start at -15. (float) rather than 15 (int)

[ZLLentz]

That's a really interesting find. We could sort of patch around this in an interface layer, probably. I wonder if this would motivate a change on the ophyd end, though I'm hesitant to do that with the eternally-impending BestEffortCallback rework.

[klauer]

A bit of a rant:
The whole "failed to format row" is really lazy on LiveTable's end. Sure, the data types may not have been compliant between rows, but your (as LiveTable) goal is to show me the information as it comes in. I don't care if the columns are not aligned this time around, but show me the data that was acquired please.

On the ophyd side, if describe() says it will give an integer, read() should not return a float. This is more of a per-signal issue that should be resolved - but something we aren't super strict about in general. We could see ophyd start enforcing data type consistency between reads. I do feel like it would be really annoying for read() to start raising.

[ZLLentz]

On the ophyd side, if describe() says it will give an integer, read() should not return a float.

It looks like what ophyd does is get the value and then do type checking on the python primitive that gets created. ref https://github.com/bluesky/ophyd/blob/aa17c1e896aa3874ff722d72c8f6d9aa59c3ebaa/ophyd/signal.py#L1257
I think, for epics at least, this only becomes a problem if the CA interface layer doesn't give us a consistent type?

So then a place this might happen more often is with sim hardware, base Signal instances, AttributeSignal, PseudoPositioner instances, etc.

I wonder if having a TypedSignal is something that would be useful from time to time- a signal that tries to cast your input value to the right type before storing it.

James's problem happened with BeamEnergyRequest which is a PV positioner... I wonder why that one exploded... perhaps because it only has a setpoint, and the signal value isn't updated with the PV's response to the put?

[klauer]

This was ambiguously-worded on my part:

On the ophyd side, if describe() says it will give an integer, read() should not return a float.

I meant more that it's the Signal's job to be consistent about its underlying data type, not that there's a bug in the describe/read mechanism.

I think, for epics at least, this only becomes a problem if the CA interface layer doesn't give us a consistent type?

In normal operating circumstances, for a Channel Access, this can really only happen if the IOC goes down and comes back up with a different type. Reasonably rare, but not impossible. The archiver appliance has issues with this as a client, requiring manual intervention in my past experience.

So then a place this might happen more often is with sim hardware, base Signal instances, AttributeSignal, PseudoPositioner instances, etc.

Those signals should probably be TypedSignals as you suggest. An ophyd-derived signal should know its type at definition-time, or at latest at initialization connection time.

James's problem happened with BeamEnergyRequest which is a PV positioner... I wonder why that one exploded... perhaps because it only has a setpoint, and the signal value isn't updated with the PV's response to the put?

Hmm, this is definitely a code path I didn't foresee... I could be wrong, but here's my take.

1. @tangkong is right about the starting value being an int or float above. It's important because of this line:
   https://github.com/bluesky/ophyd/blob/9e6c7de69a7bc6cc7e3b3c7273a5d3add9825513/ophyd/positioner.py#L233-L234
2. This is referenced by _setup_move, which happens on the first motion request, https://github.com/bluesky/ophyd/blob/9e6c7de69a7bc6cc7e3b3c7273a5d3add9825513/ophyd/positioner.py#L322
3. So bluesky probably sees the initial int value and takes that as the correct type.
4. Follow-up moves are requested as floats, so it happily accepts those

It does seem like Positioner should be casting according to the underlying data type when possible.

[ZLLentz]

I meant more that it's the Signal's job to be consistent about its underlying data type, not that there's a bug in the describe/read mechanism.

I didn't mean to suggest that you thought there was a bug in describe/read, I was just trying to work through the code path to figure out in which circumstances we could be ending up with inconsistent typing.