Crash trying to construct module_arguments when module is created in a method

[maximsch2]

Last line here crashes:

import pytorch_lightning as pl
class Module(pl.LightningModule):
    def forward(self):
        return 0

def test_outside():
    a = Module()
    print(a.module_arguments)

class A:
    def test(self):
        a = Module()
        print(a.module_arguments)

    def test2(self):
        test_outside()
        
        
test_outside() # prints {}
A().test2() # prints {}
A().test() # crashes

For context, this happens when we want to instantiate LightningModules as part of a unit testing functions.

[github-actions]

Hi! thanks for your contribution!, great first issue!

[williamFalcon]

@Borda

[Borda]

This is very strange behaviour, I can reproduce but not understand how it is possible that it misses the property...

[tullie]

It's related to the hparams frame inspection magic, right?

[williamFalcon]

another idea here haha. maybe as @Borda suggested, we make a function:

auto_register_arguments()`

that get saved to the checkpoint?

or

register_checkpoint_arg(arg)

and that way we don't need to do the magic? only optionally if the user wants it?

[Borda]

well, the question is what is safer...
against the auto-register:

• it is not very safe in case your argument name match the existing method
• while saving you need to filter all attributes to be pickable

I would stay with parsing init arguments and add an optional fund to do auto register but it on user's responsibility if he uses it or not...

[Borda]

It's related to the hparams frame inspection magic, right?

I am not sure as it properly creates the instance, I will check it again...
any suggestion about how to debug it? :]

[williamFalcon]

@maximsch2 @tullie
Fixed via #2025
This is to unblock while we revisit this API.

Mind adding other tests we should consider to make sure we don't break anything?

[awaelchli]

After spending some time with #2048 I think I know what's going on here. Since we are recursively operating on the stack frame and going up with locals.f_back we will eventually land in the e.g. test_outside() function which is not a constructor and so we have an issue, because we don't want to collect locals there. So we need to check when to stop recursion and always stay withtin the constructor. We also want to throw an error when the user calls self.auto_collect somewhere other than the init.
Pretty sure I can debug it.
Adding this to TODO list #1937 for bookkeeping

[Borda]

fixed in #2047