-
Notifications
You must be signed in to change notification settings - Fork 14
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
Float waveform PVs are displayed to the wrong precision #21
Comments
The example seems beyond the point. Your waveform record doesn't configure any display precision. If you want to display floats with two digits, configure PREC=2 on the record. The PVWS then needs to send the number with display metadata that has precision=2, but the values are simply the "Float.toString" representation, since we do want to send the actual data, and the precision is a display hint. Then make the widget use the precision to format the numbers. |
I have configured the float record to have PREC=3, e.g.
The results are similar, even if I change the caput So is the issue actually with DBWR and that it is not yet using the precision specified in the widget (or the PREC field in the case precision is |
There are two issues. The main one is in the DBWR. The "precision" metadata isn't used when formatting waveform elements. Feel free to fix that. The other one is in the PVWS. While we generally use JSON, arrays are sent as Base64 encoded binary, because that tends to reduce the packet size. We currently support "b64int" and "b64dbl". Char arrays are encoded as string (your other complaint #20), all integers are sent as b64int, and float as well as double are sent as b64dbl. The latter is why float values arrive with too many digits. Your suggested code change truncates the double values, but then still sends the float data as b64dbl, which is twice as large as a float array needs to be. The proper fix would be to add "b64flt" support, encode the float array as, well, duh, binary 32-bit IEEE float arrays. No float-to-string-back-to-double-then-b64dbl. Please add "b64flt" support to both |
Addresses discussion in Issue ornl-epics#21 to support different array types.
Thanks for the detailed feedback Kay. I have followed your suggestions and have just opened a set of PRs addressing these changes:
This looks cleaner now so I think these are worthwhile changes. However, this still does not fix the spurious precision for float arrays. Below I show the DBWR display after applying my above changes. I am comparing a waveform of type float vs a double. |
Thanks for updating PVWS and DBWR to better handle the various array data types!
Exactly. Check the basics of floating point numbers, like https://en.wikipedia.org/wiki/Single-precision_floating-point_format: In other words, you can at most have 7 significant digits for a float.
That's why almost all floating point math is now done in |
We have been testing pvws/dbwr with an EPICS waveform PV of type FLOAT, e.g.
This PV is displayed as expected in Phoebus, i.e.
However when the same BOB file is displayed in dbwr, the array values are given to a much higher precision which is misleading, i.e.
I have checked the pvws response and confirmed that this misleading array is what is sent back to the client therefore I think the issue lies in pvws. Note that we do not see this issue when using a waveform PV of type DOUBLE.
pvws tries to handle a FloatArray in the same way as it does for a DoubleArray however this doesn't seem to work. I think the problem is coming from the inherent way that floats are stored meaning that the value can change from the original (i.e.. the 'float precision issue'). I have found a way around this which minimizes the impact on the rest of the code. This includes converting the float to a string and then parsing as a double.
I have forked and created a branch with this fix - please see: main...rjwills28:pvws:float_waveform_fix. I have created a new method to handle floats based on
handleDoubles
, which handles the float values differently (see line 264+265 in above branch).The text was updated successfully, but these errors were encountered: