Defaults of optional arguments are not serialized, making every change of defaults breaking #566
Description
@stefanradev93 and I just realized that Keras' auto-config only serializes the arguments that are passed to the constructor, but not the defaults of the optional arguments. This means when we change the defaults for the optional arguments between versions, the changes will be breaking for all models where the default was not overridden in the constructor call.
Possible mitigations would be:
a) not using auto-conf: We would have to write the get_config and from_config manually, and ensure that all relevant arguments are serialized. This would be doable, but result in a lot of boilerplate and potential bugs if arguments are overlooked.
b) overwriting the auto-conf behavior to include those defaults. This would be similar to the changes in #500 and would be easy to integrate if we decide we want to take that path
c) trying to get this fixed upstream in Keras: The relevant section for the auto-conf functionality is in keras/src/ops/operation.py. It would not be difficult to adapt this, the new implementation is shown in the code snippet below:
class A(object):
def __new__(cls, *args, **kwargs):
orig_kwargs = kwargs.copy()
instance = super(A, cls).__new__(cls)
# new implementation, which also includes defaults
signature = inspect.signature(cls.__init__)
argspec = inspect.getfullargspec(cls.__init__)
try:
bound_parameters = signature.bind(None, *args, **kwargs)
# Include default values in the config.
bound_parameters.apply_defaults()
# Extract argument dictionary
kwargs = bound_parameters.arguments
# Expand variable kwargs argument
kwargs |= kwargs.pop(argspec.varkw, {})
# Remove first positional argument, self
kwargs.pop(argspec.args[0])
kwargs.pop("name", None)
if argspec.varargs is not None:
print("varargs are currently not stored")
except TypeError:
# Raised by signature.bind when the supplied args and kwargs
# do not match the signature.
pass
print("new: all arguments (incl. defaults):")
print(kwargs)
kwargs = orig_kwargs
# current implementation
arg_names = inspect.getfullargspec(cls.__init__).args
kwargs.update(dict(zip(arg_names[1 : len(args) + 1], args)))
print("current: only supplied args and kwargs:")
print(kwargs)
return instance
class B(A):
def __init__(self, x, foo="foo", **kwargs):
pass
b = B("x", bar="bar")
# new: all arguments (incl. defaults):
# {'x': 'x', 'foo': 'foo', 'bar': 'bar'}
# current: only supplied args and kwargs:
# {'bar': 'bar', 'x': 'x'}What do you think, how do we proceed best? I think being able to change defaults without breaking saved models would be important to give us flexibility while still maintaining a good upgrade experience for users.
Tagging @stefanradev93, @LarsKue, @paul-buerkner
Edit: update code to support variable kwargs