EvalResult support for val loop (PR 3/5)

[williamFalcon]

Finishes enabling EvalResult in val/test loop (still optional).

Next PR will document all of this.

Additional improvements

Also improves these things:

• .test() results now includes everything returned by the user (not just callback metrics)
• When a user wants to track a metric at the step level and epoch level, we automatically split it up:

# user wants to track the val acc for every batch and also the global val acc for the epoch
result.log('val_acc', ..., on_epoch=True, on_step=True)

# automatically becomes
`epoch_val_acc`
`step_val_acc`

[codecov]

Codecov Report

Merging #2651 into master will increase coverage by 0%.
The diff coverage is 92%.

@@           Coverage Diff           @@
##           master   #2651    +/-   ##
=======================================
  Coverage      91%     92%            
=======================================
  Files          72      74     +2     
  Lines        6129    6316   +187     
=======================================
+ Hits         5597    5786   +189     
+ Misses        532     530     -2     

[pep8speaks]

Hello @williamFalcon! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-07-22 17:31:30 UTC

[williamFalcon]

I was also thinking about that, the only annoying part is that the api for adding histograms is different for each logger, so we would have to enforce the implementation through the base class. That's probably something for a new PR.

yeah that makes sense. I guess for now we can keep global step as is? but for val metrics use this formula:
val_global_step = current_epoch * val_batch_idx

[Borda]

just crossed this, what is the case we would pass None as model?
cc: @awaelchli @justusschock @williamFalcon

[williamFalcon]

.test()

[Borda]

it is happening also in validation, well in case you call _evalaute directly with passed model

[awaelchli]

test(model=None) will take the model the trainer knows from fit, right?

[Borda]

yes but in that walk-around test, the model was not assigned to trainer...

[williamFalcon]

actually... we could do this:

val_metric/current_val_epoch

for step metrics. This will make multtiple lines on the same chart. And since val/test loaders are not shuffled, the effect will be a histogram for each batch_idx.

[williamFalcon]

but let's merge this in and we can make that change in a follow on PR