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

Floating point truncated exception when loading bag with messages with INT64 fields with values bigger than std::numerical_limits<double>::max #903

Open
mme2lr-bosch opened this issue Jan 3, 2024 · 5 comments
Assignees

Comments

@mme2lr-bosch
Copy link

mme2lr-bosch commented Jan 3, 2024

Problem description

When loading a bag which contains messages with INT64 fields whose values are bigger than std::numerical_limits<double>::max, a "Floating point truncated" execption is thrown:

Steps to reproduce (important)

  • OS Ubuntu 22.04
  • Plotjuggler 3.8.3
  • plotjuggler_msgs 0.2.2
  • plotjuggler-ros-plugins 3.8.3
  • Reproduction
    • Create bag with message which has INT64 field with value bigger than std::numerical_limits<double>::max
    • Open plotjuggler .
    • File -> Data -> Load the bag

Root Cause Analysis

When loading the bag, the function member function convert() is called for a Variant object which contains the too big INT64 value in _storage.raw_data[0]. In LOC 359, this value is converted to double using the convert_impl function with template arguments SRC equal to int64_t and DST as double.

This function is implemented in converstion_impl.hpp and checks if the conversion can be done exactly and throws a RangeException otherwise in LOC 334

Since the max value of INT64 is bigger than the max value of double, this can happen for messages with INT64 field with too big values.

Problem cannot be reproduced with dummy data

Minimal bag to reproduce problem: fake_int64_data.zip

@facontidavide
Copy link
Owner

PlotJuggler can only manage numbers that can be represented as double.

What would be the desired behavior in this case? Truncation?
There is no other solution

@mme2lr-bosch
Copy link
Author

Ok.

I think a pragmatic workaround would be to ignore the series which contain such values. This would at least allow us to visualize the rest of the bag.

@facontidavide facontidavide self-assigned this Jan 9, 2024
@facontidavide
Copy link
Owner

that will take me considerable more time than simply remove the exception.
I will do it once I have time. Feel free of course to create a PR to fix this

@mme2lr-bosch
Copy link
Author

Ok, thanks. I assume simply removing the exception is not something you would merge to master, right?

@facontidavide
Copy link
Owner

I guess I might, since I received this complaint more time than I can count.

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

No branches or pull requests

2 participants