Skip to content

Replace std::time with std::random_device as seed for random number generator#5824

Merged
mvieth merged 1 commit intoPointCloudLibrary:masterfrom
javierBarandiaran:sac_model_random_seed
Oct 2, 2023
Merged

Replace std::time with std::random_device as seed for random number generator#5824
mvieth merged 1 commit intoPointCloudLibrary:masterfrom
javierBarandiaran:sac_model_random_seed

Conversation

@javierBarandiaran
Copy link
Copy Markdown
Contributor

std::time produces the same seed during the same second. This is a problem if you need to repeat the detection multiple times consecutively.

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Sep 27, 2023

Hi, could you please explain why it is necessary to repeat the detection multiple times? On the same cloud? I am not sure I understand the use case.

@javierBarandiaran
Copy link
Copy Markdown
Contributor Author

Yes, I repeat the detection with the same cloud. The algorithm detects an initial valid solution (isModelValid returns true) with computeModel, but after optimizeModelCoefficients the obtained model is invalid (isModelValid returns false) given my constraints. Thus, I try multiple times to obtain an optimized valid model.

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Sep 28, 2023

Which model is that? We should probably take a look at that behaviour

@javierBarandiaran
Copy link
Copy Markdown
Contributor Author

javierBarandiaran commented Sep 29, 2023

It is the cylinder model. That happens because the CylinderOptimizationFunctor used in optimizeModelCoefficientsCylinder does not use the parameters constraints (Axis, EpsAngle and RadiusLimits).
I am using:
pcl::SampleConsensusModelCylinder<pcl::PointXYZ, pcl::Normal>
pcl::RandomSampleConsensus<pcl::PointXYZ>

Copy link
Copy Markdown
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Thanks!
Yeah, would we nice if we could use constrained optimization in the cylinder model (and other models), but I don't think Eigen offers that. I am just a bit surprised that some "valid" models are still "valid" after optimization and some are not.

@mvieth mvieth merged commit 9aa1336 into PointCloudLibrary:master Oct 2, 2023
@javierBarandiaran javierBarandiaran deleted the sac_model_random_seed branch October 4, 2023 09:13
@mvieth mvieth added the changelog: enhancement Meta-information for changelog generation label Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: enhancement Meta-information for changelog generation module: sample_consensus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants