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

PSO: Add param file output; PSO,CMAES: Add param rescaling py-scripts & Bugfix issue #49 #48

Merged
merged 5 commits into from
Feb 27, 2018

Conversation

hwiedPro
Copy link
Contributor

Adding python scripts that turn the unscaled optimizer outputs back into the scaled values. Afterwards they write them in a "start_parameters.yml" compatible format.
Therefore added file output of the parameters to the pso_optimizer.

@AlexanderFabisch
Copy link
Contributor

@malter This is probably something you should review. I can only say some things regarding Python code.

@@ -0,0 +1,38 @@
#!/bin/python
Copy link
Contributor

Choose a reason for hiding this comment

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

How is evalCMAESOutput.py different from evalPSOOutput.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CMAES and PSO output files have different formats. Each script matches the corresponding format.

import sys
import os
import numpy as np

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use PEP8 style for Python: https://www.python.org/dev/peps/pep-0008/

There are command line tools that you can use to check compliance.

else:
print "ERROR: Number of parameters do not match!"
else:
print "Failed. Usage: python evalCMAESOutput.py $allcmaes.dat $param.yml $output_parameters.yml"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please write code that is compatible with Python 2 and 3 because we will probably have to switch to Python 3 soon. In this case: print(...) instead of print ....

@hwiedPro
Copy link
Contributor Author

The last commit fixes issue #49

@hwiedPro hwiedPro changed the title PSO: Add param file output; PSO,CMAES: Add param rescaling py-scripts PSO: Add param file output; PSO,CMAES: Add param rescaling py-scripts & Bugfix issue #49 Feb 27, 2018
@AlexanderFabisch
Copy link
Contributor

Looks good to me. It is usually better to split this up into two pull requests. I don't see any problems with the first part of the pull request though. I will merge this now.

@AlexanderFabisch AlexanderFabisch merged commit 229f447 into rock-learning:master Feb 27, 2018
@AlexanderFabisch
Copy link
Contributor

One more thing: could you document the purpose of the scripts a little bit? You can add a comment at the beginning of the scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants