-
Notifications
You must be signed in to change notification settings - Fork 18
Add methods to print information about the evaluations #175
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
Conversation
for more information, see https://pre-commit.ci
…nto feature/print_history
AngelFP
left a comment
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.
Thanks for this! Could you implement the changes in the comments and add these two function calls to test_exploration_diagnostics.py?
| self.history_file = os.path.join(path, output_files[-1]) | ||
| self.params_file = os.path.join( | ||
| path, "exploration_parameters.json" | ||
| ) | ||
| elif path.endswith(".npy"): | ||
| exploration_dir_path = pathlib.Path(path).parent | ||
| output_file = path | ||
| params_file = os.path.join( | ||
| self.history_file = path | ||
| self.params_file = os.path.join( | ||
| exploration_dir_path, "exploration_parameters.json" | ||
| ) | ||
| else: | ||
| raise RuntimeError( | ||
| "The path should either point to a folder or a `.npy` file." | ||
| ) | ||
| exploration = self._create_exploration( | ||
| exploration_dir_path, params_file, output_file | ||
| exploration_dir_path, self.params_file, self.history_file |
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.
Is there a need to store the paths to history_file and params_file in self? They are not used anywhere else.
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.
You are right, they are not being used anywhere else yet.
Nevertheless, I think that it is important that when one initializes an instance of ExplorationDiagnostics from files, it should be known which are those files.
So the user knows exactly where this data is coming from.
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.
Right, but the path to the data is already in exploration_dir_path. Is there any reason to have the path to the json and npy files? In principle I don't see a use case.
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.
In previous versions there could be more than one history file in the exploration_dir_path.
When this is the case, and just the exploration_dir_path is given, the most recent history file is automatically selected.
Keeping the path to the selected history file seems to me a good practice so it can be known the origin of the data store in ExplorationDiagnostics.
But I see that in newer versions of optimas there is just one history file per exploration_dir_path.
So it should be sufficiently clear which one applies.
| self, | ||
| top: Optional[int] = 3, | ||
| objective: Optional[Union[str, Objective]] = None, | ||
| ) -> str: |
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.
| ) -> str: | |
| ) -> None: |
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.
Sure! thanks
|
|
||
| print() | ||
|
|
||
| def print_top_evaluations( |
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.
To keek it consistent with the other methods, I would change the name to
| def print_top_evaluations( | |
| def print_best_evaluations( |
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.
alright!
| sim_path = None | ||
| print("%20s = None" % ("dir_path")) | ||
|
|
||
| print("objective 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.
| print("objective functions:") | |
| print("objectives:") |
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.
alright too
print_best_evaluationsshows a table with the top scoring evaluations in the given objective.example:
print_evaluationshows the relevant parameters of a given evaluation.example: