Accenture / AmpliGraph

Python library for Representation Learning on Knowledge Graphs https://docs.ampligraph.org
Apache License 2.0
2.14k stars 251 forks source link

Refactoring grid search to be more generic form and adding random search as a special case #108

Closed tabacof closed 5 years ago

tabacof commented 5 years ago

Related Issue(s)

106

Description of Changes

  1. Refactoring gridsearch_next_hyperparam into something more generic that does not depend on an explicit list of parameters and registries.
  2. Adding some auxiliary functions to help with the matter of registries and unused parameters and scalar parameters.
  3. Adding the possibility of passing random callables to the param grid (e.g. "lr": lambda: np.uniform()).
  4. Adding two more arguments to support that: max_combinations and a random seed. Max combinations is mandatory only when there are random callables (otherwise all combinations will be explored).
  5. Adding more unit and integration tests to cover the newly added complexity.
  6. Improving verbose docstring to handle #79
  7. Adding the experimental history as part of the return
  8. Improving default values for arguments

Any other comments?

One important design choice is that missing parameters in the param_grid do not raise exceptions, only a warning stating the default value shall be used (discussed with @lukostaz).

Another design choice was using the same function (select_best_model_ranking) to implement both grid and random search, as special cases. The problem with two separate functions is that they are both complex and extremely similar, so there would be duplicated code and documentation and two massive functions for the user to understand.