ros2 / ros2/rosidl_python

Message equality fails with NaN values for floats/doubles

Open
#207 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
EmberScript
Stars
26
Forks
68
Avg merge
2d 13h
Merged PRs (30d)
1

Description

Bug report

Required Info:

  • Operating System:
    • Ubuntu 22.04
  • Installation type:
    • Binaries and Source
  • Version or commit hash:
    • 0.14.4
  • DDS implementation:
    • N/A
  • Client library (if applicable):
    • N/A
Steps to reproduce issue

See the commit on equality which causes the tests to fail on branch nan-failed-comparison

Expected behavior

When floating point values are set to NaN, and the == equality operator is used to check messages are equal, it should return true when two messages have the same contents

Actual behavior

NaN's don't equal themselves, and the message equality evaluates as False.

Additional information

See https://github.com/ros2/rosidl/pull/789#issuecomment-2000037932

I have a fix internally, I just need approval to share it open source.

Test Logs
21: =================================== FAILURES ===================================
21: _______________________________ test_basic_types _______________________________
21: 
21:     def test_basic_types():
21:         msg = BasicTypes()
21:     
21:         # types
21:         assert isinstance(msg.bool_value, bool)
21:         assert isinstance(msg.byte_value, bytes)
21:         assert 1 == len(msg.byte_value)
21:         # for legacy reasons, 'char' from a .msg interface maps to 'uint8'
21:         assert isinstance(msg.char_value, int)
21:         assert isinstance(msg.float32_value, float)
21:         assert isinstance(msg.float64_value, float)
21:         assert isinstance(msg.int8_value, int)
21:         assert isinstance(msg.uint8_value, int)
21:         assert isinstance(msg.int16_value, int)
21:         assert isinstance(msg.uint16_value, int)
21:         assert isinstance(msg.int32_value, int)
21:         assert isinstance(msg.uint32_value, int)
21:         assert isinstance(msg.int64_value, int)
21:         assert isinstance(msg.uint64_value, int)
21:     
21:         # default values
21:         assert msg.bool_value is False
21:         assert bytes([0]) == msg.byte_value
21:         assert 0 == msg.char_value
21:         assert 0.0 == msg.float32_value
21:         assert 0.0 == msg.float64_value
21:         assert 0 == msg.int8_value
21:         assert 0 == msg.uint8_value
21:         assert 0 == msg.int16_value
21:         assert 0 == msg.uint16_value
21:         assert 0 == msg.int32_value
21:         assert 0 == msg.uint32_value
21:         assert 0 == msg.int64_value
21:         assert 0 == msg.uint64_value
21:     
21:         # assignment
21:         msg.bool_value = True
21:         assert msg.bool_value is True
21:         msg.byte_value = b'2'
21:         assert bytes([50]) == msg.byte_value
21:         msg.char_value = 42
21:         assert 42 == msg.char_value
21:         msg.float32_value = 1.125
21:         assert 1.125 == msg.float32_value
21:         msg.float64_value = 1.125
21:         assert 1.125 == msg.float64_value
21:         msg.int8_value = -50
21:         assert -50 == msg.int8_value
21:         msg.uint8_value = 200
21:         assert 200 == msg.uint8_value
21:         msg.int16_value = -1000
21:         assert -1000 == msg.int16_value
21:         msg.uint16_value = 2000
21:         assert 2000 == msg.uint16_value
21:         msg.int32_value = -30000
21:         assert -30000 == msg.int32_value
21:         msg.uint32_value = 60000
21:         assert 60000 == msg.uint32_value
21:         msg.int64_value = -40000000
21:         assert -40000000 == msg.int64_value
21:         msg.uint64_value = 50000000
21:         assert 50000000 == msg.uint64_value
21:     
21:         # out of range
21:         with pytest.raises(AssertionError):
21:             setattr(msg, 'char_value', '\x80')
21:         for i in [8, 16, 32, 64]:
21:             with pytest.raises(AssertionError):
21:                 setattr(msg, 'int%d_value' % i, 2**(i - 1))
21:             with pytest.raises(AssertionError):
21:                 setattr(msg, 'int%d_value' % i, -2**(i - 1) - 1)
21:             with pytest.raises(AssertionError):
21:                 setattr(msg, 'uint%d_value' % i, -1)
21:             with pytest.raises(AssertionError):
21:                 setattr(msg, 'int%d_value' % i, 2**i)
21:         float32_ieee_max_next = numpy.nextafter(3.402823466e+38, math.inf)
21:         with pytest.raises(AssertionError):
21:             setattr(msg, 'float32_value', -float32_ieee_max_next)
21:         with pytest.raises(AssertionError):
21:             setattr(msg, 'float32_value', float32_ieee_max_next)
21:     
21:         # Only run bounds test on system with non-compliant IEEE 754 float64.
21:         # Otherwise the number is implicitly converted to inf.
21:         if sys.float_info.max > 1.7976931348623157e+308:
21:             float64_ieee_max_next = numpy.nextafter(1.7976931348623157e+308, math.inf)
21:             with pytest.raises(AssertionError):
21:                 setattr(msg, 'float64_value', -float64_ieee_max_next)
21:             with pytest.raises(AssertionError):
21:                 setattr(msg, 'float64_value', float64_ieee_max_next)
21:     
21:         # NaN
21:         setattr(msg, 'float32_value', math.nan)
21:         assert math.isnan(msg.float32_value)
21: >       assert msg == msg
21: E       AssertionError: assert rosidl_generator_py.msg.BasicTypes(bool_value=True, byte_value=b'2', char_value=42, float32_value=nan, float64_value=1...6_value=-1000, uint16_value=2000, int32_value=-30000, uint32_value=60000, int64_value=-40000000, uint64_value=50000000) == rosidl_generator_py.msg.BasicTypes(bool_value=True, byte_value=b'2', char_value=42, float32_value=nan, float64_value=1...6_value=-1000, uint16_value=2000, int32_value=-30000, uint32_value=60000, int64_value=-40000000, uint64_value=50000000)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the nan-failed-comparison commit and the failing test_basic_types case shown in the report. Trace the message equality path used by rosidl_generator_py for float fields, then run the relevant test to confirm that messages with the same NaN contents compare equal.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.