Add docstring for hypersearch module #84
Conversation
There was a problem hiding this comment.
After looking a bit more into the hypersearch module, I've found two things that I think are worth addressing in this PR as well:
there's a typo here:
nnfabrik/nnfabrik/utility/hypersearch.py
Line 81 in d6be40d
And another, potentially bigger issue could be:
I noticed that the user has no control over the seed at all. So if there happen to be more entires in the seed table, the Bayesian search will auto populate all seeds. I think a seed=None default argument might be helpful, so the user can set specific seed, if that's what that particular user has on his mind.
| architect (dict): Name of the contributor that added this entry | ||
| trained_model_table (dict): name (importable) of the trained_model_table | ||
| total_trials (int, optional): Number of experiments (i.e. training) to run. Defaults to 5. | ||
| arms_per_trial (int, optional): Number of different configurations used for training (for more details check https://ax.dev/docs/glossary.html#trial). Defaults to 1. |
There was a problem hiding this comment.
I think it would be helpful to have the glossary link already at the top.
There was a problem hiding this comment.
There is a link to ax at the top (in case user wants to get to know more about it), and this is pretty much the only term that is used from ax. So I am not sure how helpful adding a link at the top to glossary is. What do u imagine the sentence look like?
| @@ -8,7 +8,25 @@ | |||
|
|
|||
|
|
|||
| class Bayesian(): | |||
|
|
|||
| """ | |||
| A hyperparamter optimization tool based on Facebook Ax (https://ax.dev/), integrated with nnfabrik. | |||
There was a problem hiding this comment.
What would be helpful here I think is to include just a short sentence how it generally works. Something like: automatically adds entries to the model, dataset, and trainer tables, trains the model and stores the result in the provided TrainedModel table. So that it is immediately clear conceptually
what it is doing.
There was a problem hiding this comment.
Good point. Done.
| @@ -88,6 +159,16 @@ def _add_etery(Table, fn, config): | |||
| return fn, entry_hash | |||
|
|
|||
| def train_evaluate(self, auto_params): | |||
| """ | |||
| For a given set of parameters, add an entry to the corresponding tables, and populated the trained model | |||
There was a problem hiding this comment.
I think a sentence like this at the very top, as I mentioned, would be quite helpful!
There was a problem hiding this comment.
Addressed above.
|
Good catch about the typo - now fixed. but I completely ignored that function because it is not being used at the moment. |
|
Thanks @mohammadbashiri! I've created the issue regarding the seed: |
No description provided.