-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add postprocessing helpers for run labelling #21
Conversation
@ronaldvdv would this do what you are after for #13? I left omit_params empty by default - I guess either MIPGap, TimeLimit or both might be sensible to exclude, but it depends on what you are analyzing, so I think it would be better for the user to set this explicitly. |
* replace parameter_change with check_parameter_change * make the summary line one according to google style * remove super and make the docstring according to google style * fix the isort error * add simplex parser * fix the bug (typo) * simplify the simplex parser * adjust the tests * add barrier parser * remove the extra log example * add lp examples * add the barrier parser * add simplex parser * fix the imports * remove tests for simplex * add tests for continuous parser * add two more comments
@simonbowly I would love to have this merged, with one change suggested. The run label now includes three components (version, changed parameters, seed). Can we also have the changed parameters, formatted slightly differently ( |
@ronaldvdv yes I was playing with this last week. The code is in a completely different state from when this PR/issue was last touched, so had to be rewritten. It no longer needs to be in post-processing since the data is in a structured state before being extracted as a dataframe. Here's the behaviour:
'Label' lists non-default parameters, ignoring seed, logfile, and timelimit (Default indicates no changes). We could allow the user to customize that by providing an argument to the Does that do the trick? Do the names make sense? You just want an = sign added in the params list? |
9380b6c
to
2605c6a
Compare
Cool, I was already expecting some more hidden work behind the scenes :-D Looks good!
|
Sure, a callback makes sense. I would not go with 'ChangedParams' if it might contain anything including the version though. The use of the field is for labelling in plots, so to me Label makes sense. However, we could have the callback return a dict so you can add whatever fields you want. e.g. def label_callback(params: Dict[str, Union[str,float,int]], model_name: str, model_path: Path, version: str, seed: int):
...
return {'Label': some_label, 'NumChangedParams': some_count} and you get the fields you returned in the summary dataframe? |
Discussed with @mattmilten, and we think this is becoming a bit complex. A simpler alternative: we provide a column "ChangedParams" which contains the parameter dictionary. Then, the user can use pandas functions like |
Essentially, it would allow you to do this: >>> (
glt.parse("data/*.log").summary()
['ChangedParams'].apply(lambda d: "-".join(f"{k}={v}" for k, v in d.items() if k != 'TimeLimit')
.head()
)
0 Heuristics=0-Cuts=0
1 Heuristics=0-Cuts=0
2 Heuristics=0-Cuts=0
3 Heuristics=0.1-Cuts=0
4 Heuristics=0.1-Cuts=0
Name: ChangedParams, dtype: object which is exactly equivalent to providing a callback to |
Yes, love it! Much better to have the original data (parameter dictionary) available to the end user. |
2605c6a
to
449ceb4
Compare
Adds a 'ChangedParams' column to the summary dataframe, containing a dictionary of non-default parameter values for the run.
e52d3ec
to
aa53003
Compare
@mattmilten can you please review? This would close #13. I think a 2.1.0 can be released after merging this, there are a few unreleased changes waiting. |
Looks great, thanks! |
Adds the run_label function in the postprocessing module which builds a label based on version and changed parameters.
Example usage:
Output is a Series, the same as current "Log" column for the glass4 examples, but built from the parameter values so does not rely on the file naming convention.