Skip to content

Commit

Permalink
🐛 use fuzzy comparison for floating point in const evaluation equalit…
Browse files Browse the repository at this point in the history
…y check (#809)

## Description

This PR fixes another small bug observed as part of #803, which only
surfaced under macOS when using GCC as the compiler.
Turns out that GitHub's CodeQL was right after all and exact equality
checks on floating point values can come back to bite you.
In this case, two computations of `std::tan(1.0)` resulted in values
that were off by a single ULP.
Although I am not 100% certain how this can even happen, this PR works
around any such errors by using a fuzzy floating point comparison with
an epsilon on the order of `10-12`.

## Checklist:

<!---
This checklist serves as a reminder of a couple of things that ensure
your pull request will be merged swiftly.
-->

- [x] The pull request only contains commits that are related to it.
- [x] I have added appropriate tests and documentation.
- [x] I have made sure that all CI jobs on GitHub pass.
- [x] The pull request introduces no new warnings and follows the
project's style guidelines.
  • Loading branch information
burgholzer authored Jan 22, 2025
2 parents cefedf2 + 32f0a47 commit 9265d78
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
#include "ir/parsers/qasm3_parser/NestedEnvironment.hpp"
#include "ir/parsers/qasm3_parser/Statement.hpp"

#include <cmath>
#include <cstddef>
#include <cstdint>
#include <limits>
#include <memory>
#include <optional>
#include <string>
Expand Down Expand Up @@ -65,7 +67,8 @@ struct ConstEvalValue {
case ConstUint:
return std::get<0>(value) == std::get<0>(rhs.value);
case ConstFloat:
return std::get<1>(value) == std::get<1>(rhs.value);
return std::abs(std::get<1>(value) - std::get<1>(rhs.value)) <
std::numeric_limits<double>::epsilon() * 1024;
case ConstBool:
return std::get<2>(value) == std::get<2>(rhs.value);
}
Expand Down
12 changes: 6 additions & 6 deletions test/ir/test_qfr_functionality.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -714,10 +714,10 @@ TEST_F(QFRFunctionality, addControlStandardOperation) {
op.addControl(1);
op.addControl(2);
ASSERT_EQ(op.getNcontrols(), 2);
const auto expectedControls = Controls{1, 2};
const auto expectedControls = Controls{1U, 2U};
EXPECT_EQ(op.getControls(), expectedControls);
op.removeControl(1);
const auto expectedControlsAfterRemove = Controls{2};
const auto expectedControlsAfterRemove = Controls{2U};
EXPECT_EQ(op.getControls(), expectedControlsAfterRemove);
op.clearControls();
EXPECT_EQ(op.getNcontrols(), 0);
Expand All @@ -735,10 +735,10 @@ TEST_F(QFRFunctionality, addControlSymbolicOperation) {
op.addControl(2);

ASSERT_EQ(op.getNcontrols(), 2);
auto expectedControls = Controls{1, 2};
auto expectedControls = Controls{1U, 2U};
EXPECT_EQ(op.getControls(), expectedControls);
op.removeControl(1);
auto expectedControlsAfterRemove = Controls{2};
auto expectedControlsAfterRemove = Controls{2U};
EXPECT_EQ(op.getControls(), expectedControlsAfterRemove);
op.clearControls();
EXPECT_EQ(op.getNcontrols(), 0);
Expand All @@ -757,10 +757,10 @@ TEST_F(QFRFunctionality, addControlClassicControlledOperation) {
op.addControl(2);

ASSERT_EQ(op.getNcontrols(), 2);
auto expectedControls = Controls{1, 2};
auto expectedControls = Controls{1U, 2U};
EXPECT_EQ(op.getControls(), expectedControls);
op.removeControl(1);
auto expectedControlsAfterRemove = Controls{2};
auto expectedControlsAfterRemove = Controls{2U};
EXPECT_EQ(op.getControls(), expectedControlsAfterRemove);
op.clearControls();
EXPECT_EQ(op.getNcontrols(), 0);
Expand Down

0 comments on commit 9265d78

Please sign in to comment.