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

Fix PSO bound type #124

Merged
merged 2 commits into from Aug 11, 2019

Conversation

@zoq
Copy link
Member

commented Aug 9, 2019

Fix for issue reported in #123.

@rcurtin

rcurtin approved these changes Aug 9, 2019

Copy link
Member

left a comment

Easy fix. :)

@@ -159,16 +159,16 @@ class PSOType
size_t& NumParticles() { return numParticles; }

//! Retrieve value of lowerBound.
size_t LowerBound() const { return lowerBound; }
arma::vec LowerBound() const { return lowerBound; }

This comment has been minimized.

Copy link
@lozhnikov

lozhnikov Aug 10, 2019

Contributor

Shouldn't it be a const reference?

const arma::vec& LowerBound() const { return lowerBound; }

This comment has been minimized.

Copy link
@zoq

zoq Aug 11, 2019

Author Member

Right, updated.


//! Retrieve value of upperBound.
size_t UpperBound() const { return upperBound; }
arma::vec UpperBound() const { return upperBound; }

This comment has been minimized.

Copy link
@lozhnikov

lozhnikov Aug 10, 2019

Contributor

See the comment above.

@mlpack-bot

mlpack-bot bot approved these changes Aug 10, 2019

Copy link

left a comment

Second approval provided automatically after 24 hours. 👍

@@ -159,16 +159,16 @@ class PSOType
size_t& NumParticles() { return numParticles; }

//! Retrieve value of lowerBound.
size_t LowerBound() const { return lowerBound; }
const arma::vec LowerBound() const { return lowerBound; }

This comment has been minimized.

Copy link
@lozhnikov

lozhnikov Aug 11, 2019

Contributor

Did you forget to add a reference &?

const arma::vec&

I thought the copy-constructor of arma::vec isn't trivial.

This comment has been minimized.

Copy link
@zoq

zoq Aug 11, 2019

Author Member

You are absolutely right, thanks again for pointing this out.


//! Retrieve value of upperBound.
size_t UpperBound() const { return upperBound; }
const arma::vec UpperBound() const { return upperBound; }

This comment has been minimized.

Copy link
@lozhnikov

lozhnikov Aug 11, 2019

Contributor

See the comment above.

zoq added some commits Aug 11, 2019

@zoq zoq force-pushed the zoq:PSOBoundTypeFix branch from d1203ef to f387fad Aug 11, 2019

@lozhnikov
Copy link
Contributor

left a comment

Looks good to me.

@rcurtin

This comment has been minimized.

Copy link
Member

commented Aug 11, 2019

@lozhnikov thanks for catching the const reference issue; I completely overlooked it. :)

@rcurtin rcurtin merged commit af2a890 into mlpack:master Aug 11, 2019

2 checks passed

Static Code Analysis Checks Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.