incorrect usage of detach/cpu/to

[dvolgyes]

🐛 Bug

Incorrect use of detach() and cpu() during fixing #4592.

Please reproduce using the BoringModel

You cannot really.

To Reproduce

Use following BoringModel and post here

The fix for #4592 has good intentions but an obvious bug slipped through.

It is easy to understand but hard to test it, so let's rely on common sense:
That is the original code:

            if isinstance(output, Result):
                output.detach()
                if self.move_metrics_to_cpu:
                    output.cpu()

Expected behavior

And that supposed to be the correct code, obviously:

            if isinstance(output, Result):
                output = output.detach()
                if self.move_metrics_to_cpu:
                    output = output.cpu()

Environment

Any.

Additional context

I will make a trivial PR for this, I just wanted to create a ticket to actually track the issue.
In theory, the bug is general pattern, but in practice these two lines were the only ones affected in PL.

[dvolgyes]

I actually found a few other occasions too, see followup commits.

[Borda]

@dvolgyes great, mind sending a PR? 🐰

[dvolgyes]

I see that some tests fail, but I don't see how is this related to my changes.
Can you help me understand at least one of the test cases?
Or is it just timeout?

[edenlightning]

@Borda

[tchaton]

Dear @dvolgyes,

Result objects were doing an in-place operation. Reverted your changes on the model as it is cleaner to use them as in-place operation.
However, Result objects were just dictionaries, but it makes sense to have return self.

Best,
T.C