-
Notifications
You must be signed in to change notification settings - Fork 15
Add tests for fractionation module #279
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
base: dev
Are you sure you want to change the base?
Conversation
2aef263 to
e9ac63a
Compare
| """Class to create a faked SolutionIO object with a Gauss function.""" | ||
|
|
||
| def __init__(self, solution_struct, comp_system): | ||
| """solution struct has to be a dictionary with |
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.
Improve docstring
| ) | ||
|
|
||
|
|
||
| class GaussianPulse(SolutionIO): |
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.
Wouldn't it make sense to update the solution fixtures in test_solution.py?
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.
Consider a different name. "Fractionating" is not a really a thing. Consider what exactly you are testing here. Is it the FractionationOptimizer? Is it performance?
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.
Does this make the optimization more robust or is this just "to play around"?
Added the Cases i used for the fractionating topic as test to have a small suit that checks wheter the standard
fractionoptimizerand it's default parameter are suitable. Hopefully this shows possible instabilities with the cobyla optimizer when changing parameters.I guess the cobyla fixture is a litle bit messy.
Also changed the parameters for tol and rhoberg another time.