-
Notifications
You must be signed in to change notification settings - Fork 1
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
Added implementations of continous test functions #67
Conversation
…ons_refactor # Conflicts: # src/main.rs
…ency, added exceptions to functions that work only in a limited number of dimensions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, maybe next time import f64::consts::PI etc. and skip the namespace
Co-authored-by: Kacper Ślusarczyk <[email protected]>
Co-authored-by: Kacper Ślusarczyk <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job 👍🏻
I've left some comments below, would be nice if they could be addressed in some followup PR.
@@ -0,0 +1 @@ | |||
pub mod test_functions; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing new line at the file end.
For some reason I do not have option to "suggest changes" (I don't see such button :D), so I'm just leaving descriptions.
if x.len() < 1 { | ||
panic!("Rastrigin function takes an at least one dimensional vector as a parameter."); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be more clear to just use assert!
macro here.
This can be applied to parameter validation you did in every of the implemented functions.
panic!("Rastrigin function takes an at least one dimensional vector as a parameter."); | ||
} | ||
let mut result: f64 = 0.0; | ||
for x_curr in x.iter() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe using .iter()
explicitly here is not necessary. Compiler will deduce this on its own.
This can be applied in few more places.
let x1 = x[0]; | ||
let x2 = x[1]; | ||
let mut result: f64 = 0.0; | ||
result += f64::sin(x1 + x2); | ||
result += f64::powi(x1 - x2, 2); | ||
result += -1.5 * x1; | ||
result += 2.5 * x2; | ||
result += 1.0; | ||
return result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we can omit creating result variable completely here.
let mut result: f64; | ||
result = -1.0 * f64::abs(f64::sin(x1) * f64::cos(x2) * f64::exp(f64::abs(1.0 - f64::sqrt(f64::powi(x1, 2) + f64::powi(x2, 2)) / f64::consts::PI))); | ||
return result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can omit creating result variable here, and in few more places.
Description
Added implementations of continous test functions
Linked issues
closes #17