Support multiple `eval` datasets about composer HOT 5 CLOSED

Current Plan (looking for feedback)

• Create an EvalDataset (better name suggestion?) data structure to store at least a dataloader (+ relevant Hparams) and a list of metrics for each evaluation dataset.
• Trainer is passed an EvalDataset or a List[EvalDataset]

Questions + Fields that need changing

• compute_training_metrics (trainer) - since metrics would become EvalDataset specific, would need to store which metrics are going to be used for these operations
• Do evaluation parameters like eval_batch_size change for different datasets? Should this also be a part of EvalDataset
• For validating every N batches, would it be helpful to store a flag in EvalDataset that determines whether a particular dataset is used? (default=True)

from composer.

I agree with the need for an EvalDataset (suggestion, rename to Evaluator) that stores more information than the current val_dataset hparams object does. In general, I like the idea of things like validate_every_n_batches being moved into this object, as long as we can continue to make it reasonable to use our trainer without getting hparams involved (something I'm about to take a stab at). The main question I have is how would the metrics function of the model determine which Evaluator is invoking it (and which metrics to calculate); I think it would be OK for metrics to take a second optional parameter with the ID of the invoking Evaluator.

Also hoping to see feedback from @ajaysaini725, who IIRC contributed significantly to the metrics code before launch.

from composer.

Could we perhaps move the metrics and validate functions inside the evaluator and remove it from the model? Or we could use model metrics as defaults if none are specified by an Evaluator so a user can optionally just specify the metrics once? @jbloxham @ajaysaini725

from composer.

A few things to consider:

• The output of model.validate(eval_data_batch) -> Tuple[Any, Any] gets passed straight into each metrics returned by model.metrics() so the validate() function needs to be compatible with all EvalDatasets. Also, we should definitely keep validate() as a part of the model definition because it relies on the forward pass logic of the model.
• Ideally we don't want to specify anything data-related in the model class because the model class definition should be an isolated unit that works across many different training runs. It's better to have each EvalDataset specify the metrics it needs. The model class definitions should still have a metrics() function that returns all metrics which can be computed for the model and then each EvalDataset can specify exactly which metrics are relevant for that dataset (this can be done in DatasetHparams and also should be stored in DataloaderSpec so that it works with the Trainer __init__. Having the relevant metrics for a model be returned by the model.metrics() function prevents an EvalDataset from specifying a metric that doesn't work for a particular model (i.e. the model.metrics() specifies all metrics for a model and then EvalDataset specifies the subset of metrics to use for that dataset). If EvalDataset doesn't specify anything then the default is to use all metrics.

Also, another thought: what if we move the train_dataset and eval_dataset to be parameters of Trainer.fit() rather than parameters of Trainer instantiation?

from composer.

Makes sense - I like the idea of making the datasets parameters to the fit function

from composer.