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.

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

seed for sampling joint states in KinematicTree #487

Merged
merged 2 commits into from
Jan 9, 2019

Conversation

christian-rauch
Copy link
Collaborator

Method SetSeed for setting seed of random number generator in KinematicTree.

Copy link
Collaborator

@wxmerkt wxmerkt left a comment

Choose a reason for hiding this comment

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

Can you please add doxygen and perhaps a getter?

@christian-rauch
Copy link
Collaborator Author

The std::mt19937 does not store the seed it was initialised with. Or at least there is no public method/member to access this.
We would need to store the seed as a member of KinematicTree and this would also not be defined if SetSeed wouldn't have been called before.

@wxmerkt
Copy link
Collaborator

wxmerkt commented Jan 9, 2019

Alright - can you please change to C++ style doxygen (///) and then we are good to merge :)

@christian-rauch
Copy link
Collaborator Author

I didn't know that there are different doxygen styles per language.
QtCreator is actually using this style per default. E.g. you type /** + Enter and it creates the template automatically. In this case:

/**
 * @brief SetSeed
 * @param seed
 */

I very like this feature because automatically includes all parameters and the return value of a method.

Do you insist on the triple slash (///) style? :-)

@wxmerkt
Copy link
Collaborator

wxmerkt commented Jan 9, 2019

I like the same auto-completion feature and would be happy with either - Vlad does insist ;)

@christian-rauch
Copy link
Collaborator Author

Oh well, here you go :-)

Copy link
Collaborator

@wxmerkt wxmerkt left a comment

Choose a reason for hiding this comment

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

Sorry, just had a closer look after the meeting with two minor comments (language and pass by reference)

exotica_core/include/exotica_core/kinematic_tree.h Outdated Show resolved Hide resolved
@christian-rauch christian-rauch force-pushed the kinematic_tree_seed branch 3 times, most recently from ae64d63 to 99cca20 Compare January 9, 2019 17:05
@christian-rauch
Copy link
Collaborator Author

This should be fine now.

@wxmerkt wxmerkt merged commit d50cc5d into ipab-slmc:master Jan 9, 2019
@wxmerkt
Copy link
Collaborator

wxmerkt commented Jan 9, 2019

Thank you very much :-)

@christian-rauch christian-rauch deleted the kinematic_tree_seed branch January 10, 2019 10:58
wxmerkt added a commit that referenced this pull request Jun 22, 2020
seed for sampling joint states in KinematicTree
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants