PointCloudLibrary/pcl

Inconsistent handling of default seed in NormalGenerator & UniformGenerator

Open

#2,985 opened on Apr 9, 2019

View on GitHub
 (3 comments) (1 reaction) (0 assignees)C++ (9,023 stars) (4,506 forks)batch import
effort: lowgood first issuekind: todoneeds: feedback

Description

As already discussed here default seed in random.h is not handled consistently.

https://github.com/PointCloudLibrary/pcl/blob/b4c785bf32b392b4570201e8fe9f97395bd98ba1/common/include/pcl/common/random.h#L163-L186

One constructor of NormalGenerator has as default seed -1 during the other one (via Parameters) has 1 as default seed.

Furthermore you get currently following warning:

common/include/pcl/common/impl/random.hpp:49:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if(parameters_.seed != -1)

So we should discuss how to adjust code there.

Contributor guide

Tech stack
cpp
Domain
backend
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
under 1 hour
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
C++familiarity with PCL common module
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
70
Research direction
Examine the discussion in PR #2956 and the linked lines in random.h (L163-186) and random.hpp (L49). The problem is inconsistent default seed values ( 1 vs 1) and a signed/unsigned comparison warning. The fix involves choosing a consistent default (likely 0) and adjusting the comparison to use the same type. Also consider whether seed 1 should have special meaning.