-
Notifications
You must be signed in to change notification settings - Fork 8
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
[refactoring] Shortening the experiment names #47
base: main
Are you sure you want to change the base?
Conversation
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.
@qetdr great work! I very much appreciate the thoughts you put into renaming and like the idea of abbreviating all names. Also, thanks for the detailed description of the PR changes, that makes it easy to understand your code.
The first part of your changes looks good to me. Have another look at the part, where you rename the model parameters. Here we need to have a solution that generalizes well to all possible model parameters.
As some inspiration: Another idea for naming the experiments would be with a numeric unique identifier. However, in this case, we would still need to hold the option for the user, to view or query the underlying configuration. What are your thoughts on this?
Lastly, please check the following:
- black and flake8 is still failing in your workflows
- regarding your example
'dat-NePrM-SM_mult-LR_0.1-_data_params_-SM_add-freq_MS'
, where does the_
after_data_param
come from?
Thank you!!
abbreviated_name += "M" # for 'Model' | ||
|
||
# 3. params (dictionary with abbreviations) | ||
d = {'seasonality_mode': 'SM', |
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.
The model parameter names can largely vary among the available tsf model. Hence, it might not be a good idea of hard-coding them in a dict. Can you think of another solution?
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 should find a solution, that generalizes well to all possible model parameters
🔬 Background
Each experiment within a benchmark is given a name. This name is created based on the model name, the dataset set, and the selected parameters. While this is mostly a unique name, it is also very long. Is there a way to adapt the experiment name to be short and unique? Implement an improved experiment name.
🔮 Key changes
The main change was abbreviating the object names in the experiment name.
parameter:argument
->PM_arg
).Example of the change:
'datasetname_NeuralProphet_seasonality_mode_multiplicative_learning_rate_0.1__data_params__seasonality_mode_ additive, freq_ MS_'
to
'dat-NePrM-SM_mult-LR_0.1-_data_params_-SM_add-freq_MS'
📋 Review Checklist