serialization: apply new scheme for package (breaking change)

[vpratz]

Closes #451.

• introduces new policy for consistent naming for serilization (see [BUG] serializable decorator produces wrong package for serialization #451
  for a discussion): standard is the path of a module a class resides
  in, truncated at depth two. So for a class in bayesflow.networks, we
  set package="bayesflow.networks", even if they live in the
  bayesflow.networks.mlp submodule.
• The serializable decorator checks this and errors if this is not
  followed. The check can be disabled for certain cases (e.g., classes
  in the experimental module, that might eventually live somewhere
  else).
• After this commit, previously saved models will not be loadable. As we
  introduced a bug regarding this anyway ([BUG] serializable decorator produces wrong package for serialization #451), we will accept this and
  should inform users about it.
• usage of direct calls to keras.saving.register_keras_serializable
  were replaced with our custom decorator.
• update docs and add a disclaimer to README regarding potential breaking changes until 2.1 release (feel free to adapt)

Todo for next patch release

• as this breaks loading models from the previous patch versions, make a brief announcement in our community channels, so that people are aware and can pin their installation, if necessary
• include changes from Add projectors to DeepSet #453 (if we want to include them), which are breaking as well

[codecov]

Codecov Report

Attention: Patch coverage is 95.19231% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
bayesflow/experimental/resnet/dense_resnet.py	0.00%	1 Missing ⚠️
bayesflow/experimental/resnet/double_conv.py	0.00%	1 Missing ⚠️
bayesflow/experimental/resnet/double_linear.py	0.00%	1 Missing ⚠️
bayesflow/experimental/resnet/resnet.py	0.00%	1 Missing ⚠️
bayesflow/utils/serialization.py	85.71%	1 Missing ⚠️

Files with missing lines	Coverage Δ
bayesflow/adapters/adapter.py	83.73% <100.00%> (ø)
bayesflow/adapters/transforms/as_set.py	92.30% <100.00%> (ø)
bayesflow/adapters/transforms/as_time_series.py	92.30% <100.00%> (ø)
bayesflow/adapters/transforms/broadcast.py	82.97% <100.00%> (ø)
bayesflow/adapters/transforms/concatenate.py	87.69% <100.00%> (ø)
bayesflow/adapters/transforms/constrain.py	83.49% <100.00%> (ø)
bayesflow/adapters/transforms/convert_dtype.py	100.00% <100.00%> (ø)
bayesflow/adapters/transforms/drop.py	88.23% <100.00%> (ø)
...sflow/adapters/transforms/elementwise_transform.py	78.94% <100.00%> (ø)
bayesflow/adapters/transforms/expand_dims.py	100.00% <100.00%> (ø)
... and 76 more

... and 1 file with indirect coverage changes

[LarsKue]

We should think about whether we want the current module check to be a flag in the serializable decorator. If users are supposed to use our pipeline rather than the one from keras, the check must be off by default.

I think one of the following options is better:

1. We add a check in any test_serialize_deserialize for the module structure, but remove the flag from the decorator altogether.
2. We leave the flag in the current serializable decorator, but turn it off by default. Internal modules can use something like from bayesflow.internal import serializable which is a partial(bayesflow.utils.serializable, disable_module_check=False)

I am in favor of 1, since it avoids the issue of 2 that new contributors may accidentally use the "wrong" kind of decorator.

[vpratz]

Thanks for your comments. I like the idea of moving the test to the test suite, but as far as I can tell (correct me if I'm wrong) the test_serialize_deserialize test only explicitly covers a subset of classes. Many others are implicitly part of the test, but I do not know if we can access those for the test we have in mind.
I agree with your concern regarding 2.
A third option would be to limit the check to the case where the module name indicates a class is part of BayesFlow. I will add a commit that implements this, but we can still decide to go with one of the other options.

Would you say that we actually want users to to use our decorator instead of the Keras-provided one? Currently the only thing it does is to supply this check, which is only relevant internally, so we could also think about making it private. Or do we expect to encounter future scenarios where we want to have additional functionality in this decorator? As we cannot rule out that users use the Keras decorator instead, I think this would not be so desirable anyways.

What do you think?

[LarsKue]

Thanks for the update. I think users should use our decorator, as this gives us the most flexibility in terms of maneuvering around keras.