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.

Sign up for GitHub

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

Platform specific #ifs #1966

Merged
merged 11 commits into from
Jan 10, 2025
Merged

Platform specific #ifs #1966

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
72da68e
platform specific #ifs
varunagrawal Jan 8, 2025
c00f663
other tests
varunagrawal Jan 8, 2025
212c792
support Windows
varunagrawal Jan 8, 2025
d9106fd
add numeric header for std::accumulate
varunagrawal Jan 8, 2025
41280d4
update tests that failed and add shonan averaging test
varunagrawal Jan 8, 2025
d4726cf
fix some regressions
varunagrawal Jan 9, 2025
cb2be68
fix test with better precision
varunagrawal Jan 9, 2025
556ed9b
separate Windows regression for Shonan Averaging
varunagrawal Jan 9, 2025
438bead
small cleanup
varunagrawal Jan 9, 2025
fbe4ad1
common Apple and Windows assertions
varunagrawal Jan 10, 2025
62b0ed6
change from ifdef to if
varunagrawal Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 24 additions & 13 deletions gtsam/hybrid/tests/testHybridBayesNet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <CppUnitLite/TestHarness.h>

#include <memory>
#include <numeric>

using namespace std;
using namespace gtsam;
Expand Down Expand Up @@ -552,19 +553,29 @@ TEST(HybridBayesNet, Sampling) {
EXPECT_LONGS_EQUAL(2, average_continuous.size());
EXPECT_LONGS_EQUAL(num_samples, discrete_samples.size());

// Regressions don't work across platforms :-(
// // regression for specific RNG seed
// double discrete_sum =
// std::accumulate(discrete_samples.begin(), discrete_samples.end(),
// decltype(discrete_samples)::value_type(0));
// EXPECT_DOUBLES_EQUAL(0.477, discrete_sum / num_samples, 1e-9);

// VectorValues expected;
// expected.insert({X(0), Vector1(-0.0131207162712)});
// expected.insert({X(1), Vector1(-0.499026377568)});
// // regression for specific RNG seed
// EXPECT(assert_equal(expected, average_continuous.scale(1.0 /
// num_samples)));
// regression for specific RNG seed
double discrete_sum =
std::accumulate(discrete_samples.begin(), discrete_samples.end(),
decltype(discrete_samples)::value_type(0));
#if __APPLE__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t see differences between these different paths??

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using the CI to understand the differences in the results since I can avoid instrumenting individual platform runs manually. I'll request a review when this is ready.

EXPECT_DOUBLES_EQUAL(0.477, discrete_sum / num_samples, 1e-9);
#elif __linux__
EXPECT_DOUBLES_EQUAL(0.477, discrete_sum / num_samples, 1e-9);
#elif _WIN32
EXPECT_DOUBLES_EQUAL(0.477, discrete_sum / num_samples, 1e-9);
#endif

VectorValues expected;
expected.insert({X(0), Vector1(-0.0131207162712)});
expected.insert({X(1), Vector1(-0.499026377568)});
// regression for specific RNG seed
#if __APPLE__
EXPECT(assert_equal(expected, average_continuous.scale(1.0 / num_samples)));
#elif __linux__
EXPECT(assert_equal(expected, average_continuous.scale(1.0 / num_samples)));
#elif _WIN32
EXPECT(assert_equal(expected, average_continuous.scale(1.0 / num_samples)));
#endif
}

/* ****************************************************************************/
Expand Down
15 changes: 12 additions & 3 deletions gtsam/linear/tests/testGaussianBayesNet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,18 @@ TEST(GaussianBayesNet, sample) {
std::mt19937_64 rng(4242);
auto actual3 = gbn.sample(&rng);
EXPECT_LONGS_EQUAL(2, actual.size());
// regression is not repeatable across platforms/versions :-(
// EXPECT(assert_equal(Vector2(20.0129382, 40.0039798), actual[X(1)], 1e-5));
// EXPECT(assert_equal(Vector2(110.032083, 230.039811), actual[X(0)], 1e-5));

// regressions
#ifdef __APPLE__
EXPECT(assert_equal(Vector2(20.0129382, 40.0039798), actual[X(1)], 1e-5));
dellaert marked this conversation as resolved.
Show resolved Hide resolved
EXPECT(assert_equal(Vector2(110.032083, 230.039811), actual[X(0)], 1e-5));
#elif __linux__
EXPECT(assert_equal(Vector2(20.0129382, 40.0039798), actual[X(1)], 1e-5));
EXPECT(assert_equal(Vector2(110.032083, 230.039811), actual[X(0)], 1e-5));
#elif _WIN32
EXPECT(assert_equal(Vector2(20.0129382, 40.0039798), actual[X(1)], 1e-5));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as Apple?

EXPECT(assert_equal(Vector2(110.032083, 230.039811), actual[X(0)], 1e-5));
#endif
}

/* ************************************************************************* */
Expand Down
10 changes: 8 additions & 2 deletions gtsam/linear/tests/testGaussianConditional.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,14 @@ TEST(GaussianConditional, sample) {
std::mt19937_64 rng(4242);
auto actual3 = conditional.sample(given, &rng);
EXPECT_LONGS_EQUAL(1, actual2.size());
// regression is not repeatable across platforms/versions :-(
// EXPECT(assert_equal(Vector2(31.0111856, 64.9850775), actual2[X(0)], 1e-5));
// regressions
#ifdef __APPLE__
EXPECT(assert_equal(Vector2(31.0111856, 64.9850775), actual2[X(0)], 1e-5));
dellaert marked this conversation as resolved.
Show resolved Hide resolved
#elif __linux__
EXPECT(assert_equal(Vector2(31.0111856, 64.9850775), actual2[X(0)], 1e-5));
#elif _WIN32
EXPECT(assert_equal(Vector2(31.0111856, 64.9850775), actual2[X(0)], 1e-5));
#endif
}

/* ************************************************************************* */
Expand Down
Loading