[tune] Incorrect end of trial increment for function API

[richardliaw]

Seems to be an indexing problem when running experiments with the functional API. From the mailing list:

I'm running with a simple trainable function (so no Trainable sub-classing) but I still seem to have every trial run have '2' as the iteration count, e.g. TERMINATED [pid=3942], 327 s, 2 iter <--- how is this possible? Surely there's only 'one' iteration per trial to be done if I'm not doing any checkpointing etc?

[richardliaw]

Proposed fix for this is to improve documentation (no way to detect otherwise).

[gehring]

Could this be caused by the fact that logging faster that the function API will only log at most once per second, dropping logs given to the reporter if logging faster than that?

[richardliaw]

Sort of but not exactly; basically the last_result is provided twice (once when invoked, and then once again when the StatusReporter._done attribute is set to True. There are cases where if the reporter ends quickly enough such that last_result is only provided once.

I think the main workaround here is that the user should set done when the training completes.

[gehring]

This question might be better in a standalone issue, but why is the function runner implementation so complex? Is there a reason why a multi-threaded solution with sleeps was used?

I think I might be able to propose/contribute an alternative design that would unify the logging behavior of the function API and the trainable API (and simplify function_runner.py) but I want to make sure I understand a bit better what led to this implementation.

[richardliaw]

@gehring Sorry missed your message - the function runner allows users to quickly integrate their original training loops into Tune without restructuring their code and only adding a reporter call. Does that make sense? Let me know what you had in mind

[gehring]

@richardliaw Thanks for getting back to me! First, let me say that I am a big fan of the function runner API and approach. The API is simple and powerful. However, there is a lot of complexity in the runner that end up adding a lot of counter intuitive behavior which I believe to be easy to fix but not without completely re-writing the runner.

The simplest but most significant change would be to drop the reporter all together and go for a generator based solution. If we have users yield a dict of results in the same way a Trainable would return, then wrapping an arbitrary function in a Trainable becomes pretty simple. Below is a skeleton of what I had in mind.

The function's api remains simple, only removing the reporter:

def trainable(config):
  """
  Args:
      config (dict): Parameters provided from the search algorithm
          or variant generation.
      reporter (Reporter): Handle to report intermediate metrics to Tune.
  """

  while True:
      # ...
      yield my_results

The above example should cover most (if not all) the intended use cases for the function API. The function runner is straightforward. Copying the overriding of _trainable_func approach from the current runner, we have:

class FunctionRunner(Trainable):

  def _setup(self, config):
    self._func_config = config.copy()
    self._generator = None
    
  def _trainable_func(self):
    """Subclasses can override this to set the trainable func."""
    
    raise NotImplementedError
    
  def _train(self):
    if self._generator is None
      self._generator = self._trainable_func(self._func_config)
    try:
      result = next(self._generator)
      return result
    except StopIteration:
      return {"done": True}

Let me know if there are some use cases I might be forgetting. I'd be happy to contribute a fully fleshed out implementation if this is a direction you are interested in pursuing!

EDIT: fixed runner to using the return statement based logic of Trainable. I accidentally used it as a generator.

[richardliaw]

Hm, this looks promising! I think the only thing is seeing whether or not you can actually serialize the generator and pass it around to multiple processes/machines. That being said, if there's no weird gotchas, a prototype would be awesome! It looks way cleaner (and obviously we would want to maintain support for the reporter and Trainable for backwards compat). Richard

…

On Mon, Feb 4, 2019 at 1:30 PM gehring ***@***.***> wrote: @richardliaw <https://github.com/richardliaw> Thanks for getting back to me! First, let me say that I am a big fan of the function runner API and approach. The API is simple and powerful. However, there is a lot of complexity in the runner that end up adding a lot of counter intuitive behavior which I believe to be easy to fix but not without completely re-writing the runner. The simplest but most significant change would be to drop the reporter all together and go for a generator based solution. If we have users yield a dict of results in the same way a Trainable would return, then wrapping an arbitrary function in a Trainable becomes pretty simple. Below is a skeleton of what I had in mind. The function's api remains simple, only removing the reporter: def trainable(config): """ Args: config (dict): Parameters provided from the search algorithm or variant generation. reporter (Reporter): Handle to report intermediate metrics to Tune. """ while True: # ... yield my_results The above example should cover most (if not all) the intended use cases for the function API. The function runner is straightforward. Copying the overriding of _trainable_func approach from the current runner, we have: class FunctionRunner(Trainable): def _setup(self, config): self._func_config = config.copy() def _trainable_func(self): """Subclasses can override this to set the trainable func.""" raise NotImplementedError def _train(self): for result in self._trainable_func(self._func_config): if result.get("done"): return result else: yield result Let me know if there are some use cases I might be forgetting. I'd be happy to contribute a fully fleshed out implementation if this is a direction you are interested in pursuing! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3834 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEUc5RnPtWYtZIIDkL09K_OwleANYfQjks5vKKZngaJpZM4aP9Vc> .

[gehring]

Great! I opened a standalone issue (#3956) so we can close this one. I will probably be able to start writing a prototype this week. I wouldn't expect serialization to be more harder than what it is in the current solution so hopefully I don't encounter any weird issues!