Remove NaNs from loss in LRFinder

[rohitgr7]

Before submitting

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure to update the docs?
• Did you write any new necessary tests?
• If you made a notable change (that affects users), did you update the CHANGELOG?

What does this PR do?

Fixes #1850.

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

[awaelchli]

Looks good. I wouldn't know how to test this however. Which kind of models have this behaviour?
Let's also add a changelog entry?

[awaelchli]

notifiying also @SkafteNicki

[SkafteNicki]

Nan can happens at the very end of the search if the loss/gradient explodes too much before we stop the search. I think we could add a simple test, something like

lr_finder = trainer.lr_find(...)
lr_before_nan = lr_finder.suggestion()
lr_finder.results['loss'][-1] = np.nan
lr_after_nan = lr_finder.suggestion()
assert lr_before_nan == lr_after_nan

Do simple run, run the suggestion algorithm, introduce a nan artificially, recompute suggestion. If this works the two suggestions should be the same.

[Borda]

just a note for speed, could we do the filtering in PyTorch not converting to NumPy?

[SkafteNicki]

@Borda the values are stored as a list (of floats), so i guess that the penalty of converting to numpy vs pytorch is small

[williamFalcon]

the conversion is at the full end of the lr finder and before training begins no?
this means the cost is negligible unless i'm missing something

[williamFalcon]

Is this ready?

[rohitgr7]

Is this ready?

I will add the test case now.

[mergify]

This pull request is now in conflict... :(

[justusschock]

@SkafteNicki
https://github.com/rohitgr7/pytorch-lightning/blob/fix_lr_finder/pytorch_lightning/trainer/lr_finder.py#L383 still introduces gpu_syncs. Could the same behavior be achieved with tensors as well to avoid gpu-syncs? (can probably be neglected, just asking, while we're on it :))

Otherwise this looks fine to me :)