-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[tests] One test failing on OpenBSD #6178
Comments
@thyssentishman Hi, that is interesting, thanks. My best guess is that this happens because the filter uses |
Hi @mvieth. Thanks for the quick reply. I was hoping that was the issue and applied your patch from #6179 (see below) with no success. Index: filters/include/pcl/filters/random_sample.h
--- filters/include/pcl/filters/random_sample.h.orig
+++ filters/include/pcl/filters/random_sample.h
@@ -135,7 +135,7 @@ namespace pcl
inline float
unifRand ()
{
- return (static_cast<float>(rand () / static_cast<double>(RAND_MAX)));
+ return (static_cast<float>(std::rand () / static_cast<double>(RAND_MAX)));
//return (((214013 * seed_ + 2531011) >> 16) & 0x7FFF);
}
};
@@ -226,7 +226,7 @@ namespace pcl
inline float
unifRand ()
{
- return (static_cast<float> (rand () / static_cast<double>(RAND_MAX)));
+ return (static_cast<float> (std::rand () / static_cast<double>(RAND_MAX)));
}
};
} Below the compiler I'm using:
OpenBSD's rand(3) man page says that
Could this be the source of the issue? |
@thyssentishman Oh okay. I was not aware that OpenBSD does that. Yes, that definitely sounds like it could cause this issue. Regarding the RandomSample filter:
|
@mvieth I understand and I agree that this is probably not good when one is expecting reproducibility of results (as we are experiencing here). It might be good if you could bring this up for discussion on OpenBSD's tech@ mailing lists if you like. Regarding your proposed solutions, I was expecting Patch: Index: filters/include/pcl/filters/impl/random_sample.hpp
--- filters/include/pcl/filters/impl/random_sample.hpp.orig
+++ filters/include/pcl/filters/impl/random_sample.hpp
@@ -63,7 +63,11 @@ pcl::RandomSample<PointT>::applyFilter (Indices &indic
removed_indices_->resize (N - sample_size);
// Set random seed so derived indices are the same each time the filter runs
+#ifdef __OpenBSD__
+ srand_deterministic (seed_);
+#else
std::srand (seed_);
+#endif
// Algorithm S
std::size_t i = 0; |
@thyssentishman Can you try applying the same change at https://github.com/PointCloudLibrary/pcl/blob/master/filters/src/random_sample.cpp#L120 ? |
@mvieth yep, that did it, thank you very much :) Would you like me to create a PR or would you amend #6179? Or should I just keep these patches local to the port?
Just a quick note, I seem to be getting inconsistent results with test |
@thyssentishman I can amend #6179
Yes, I think so. Do you have a log of a failure? |
Sure, here is one:
|
My first guess is that this is again related to random number generation, see here: https://github.com/PointCloudLibrary/pcl/blob/master/test/registration/test_registration.cpp#L169 |
@mvieth wouldn't seeding those |
It only worked so far because it was never tested with different random seeds. On OpenBSD, where rand() gives truly random numbers (instead of repeatable, pseudo-random numbers), this test fails occasionally. The test now uses a real-world point cloud instead of a few random points, which is arguably more realistic. See PointCloudLibrary#6178
It only worked so far because it was never tested with different random seeds. On OpenBSD, where rand() gives truly random numbers (instead of repeatable, pseudo-random numbers), this test fails occasionally. The test now uses a real-world point cloud instead of a few random points, which is arguably more realistic. See PointCloudLibrary#6178
It only worked so far because it was never tested with different random seeds. On OpenBSD, where rand() gives truly random numbers (instead of repeatable, pseudo-random numbers), this test fails occasionally. The test now uses a real-world point cloud instead of a few random points, which is arguably more realistic. See #6178
I have a finished port of pcl on OpenBSD and all tests are passing except the following one (apologies for the long code-block, for some reason the
<details>
block wasn't working):Any clues? I'm compiling pcl version 1.14.1
The text was updated successfully, but these errors were encountered: