Closed vinnamkim closed 4 months ago
There isn't any bug here. The only thing that jsonargparse
knows is that the default of optimizer
is an instance of MyOptimizerCallable
. It doesn't know how this instance was created. And the error is because the instance is anonymous, only created in the class signature. It is not possible to import that object from that module. For a callable to be importable, it would need to be a function defined directly in the module, as:
def my_default_optimizer_callable(params) -> Optimizer: ...
or be an instance defined directly in the module, as:
my_default_optimizer_callable = MyOptimizerCallable(lr=0.2)
Then the parameter defined as optimizer: OptimizerCallable = my_default_optimizer_callable
. But, when defined like this, the printed config will not be as above. It will not have class_path
and init_args
, but just the import path of the callable.
Also note that using class instances as defaults is discouraged. See class-type-defaults to understand why. If you use lazy_instance
, then jsonargparse
would be able to know how to create the instance and the printed config would be like the one above.
Hi @mauvilsa,
Thanks for your kind explanation. From your guide, it seems a best practice to use lazy_instance
for this problem. However, I faced this issue to apply lazy_instance
to my callable class.
# test.py
from jsonargparse import ActionConfigFile, ArgumentParser, lazy_instance
...
class MyModel:
def __init__(self, optimizer: OptimizerCallable = lazy_instance(MyOptimizerCallable(lr=0.2))):
self.optimizer = optimizer
...
model = MyModel()
The instantiation (model = MyModel()
) in Python code produces the following error.
ValueError: Problem with given class_path '__main__.MyModel':
'LazyInstance_MyOptimizerCallable' object has no attribute 'lr'
This seems that MyModel.__call__()
cannot do lazy initialization correctly. I could manage it by adding this overriding to
https://github.com/omni-us/jsonargparse/blob/2bc81ea1a8c44856405bfcd3f6ea3feca3a57dbb/jsonargparse/_typehints.py#L1286
class LazyInitBaseClass:
...
def __call__(self, *args, **kwargs):
if call := self.__dict__.get("__call__", None):
return call(*args, **kwargs)
Can we regard this as a bug?
In the description it says you are using v4.27.6, which is not the latest. If so, please upgrade and try again. It could be already fixed by #473.
In the description it says you are using v4.27.6, which is not the latest. If so, please upgrade and try again. It could be already fixed by #473.
I upgraded it to 4.27.7
but I still got an error for this script
from jsonargparse import ActionConfigFile, ArgumentParser, lazy_instance
from lightning.pytorch.cli import OptimizerCallable
from torch.optim import SGD
from torch import nn
class MyOptimizerCallable:
def __init__(self, lr: float = 0.1):
self.lr = lr
def __call__(self, params) -> SGD:
return SGD(params, lr=self.lr)
class MyModel:
def __init__(self, optimizer: OptimizerCallable = lazy_instance(MyOptimizerCallable, lr=0.2)):
model = nn.Linear(10, 10)
self.optimizer = optimizer(model.parameters())
print(self.optimizer)
parser = ArgumentParser()
parser.add_argument("--model", type=MyModel)
parser.add_argument("--config", action=ActionConfigFile)
cfg = parser.parse_args()
print(parser.instantiate_classes(cfg))
print(MyModel())
$ python test5.py --model __main__.MyModel
SGD (
Parameter Group 0
dampening: 0
differentiable: False
foreach: None
lr: 0.2
maximize: False
momentum: 0
nesterov: False
weight_decay: 0
)
Namespace(model=<__main__.MyModel object at 0x74bfef32b790>, config=None)
Traceback (most recent call last):
File "/home/vinnamki/otx/training_extensions/test5.py", line 29, in <module>
print(MyModel())
^^^^^^^^^
File "/home/vinnamki/otx/training_extensions/test5.py", line 19, in __init__
self.optimizer = optimizer(model.parameters())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/vinnamki/otx/training_extensions/test5.py", line 13, in __call__
return SGD(params, lr=self.lr)
^^^^^^^
AttributeError: 'LazyInstance_MyOptimizerCallable' object has no attribute 'lr'
Okay, I need to look at this.
This is a bug. But the fix can't be like suggested in https://github.com/omni-us/jsonargparse/issues/481#issuecomment-2030932435, since this would make all lazy instances callable, which shouldn't be the case. I will push a fix in the next days.
@vinnamkim I have pushed a fix in #487. Please try it out.
🐛 Bug report
Same as title. Please see the following reproducible Python script.
To reproduce
Then, execute this Python script such as
Expected behavior
Expect the following configuration
However, the actual result is
Environment
pip install jsonargparse[all]
):pip install jsonargparse