-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
Hello dear friends,
I would like to raise a (small) complaint, upgrading and improving the library is good and I am really grateful for it, you do an amazing work here. However, it would be better not to change the API without warning.
I will give a few examples:
1. Layer API Change
As it is an absolute central class, one change here are leading to changes everywhere.
If any modification is done here, it should be done with a deprecation warning.
## Before
layer = tl.layers.BatchNormLayer(layer = layer)
layer = tl.layers.PReluLayer(layer = layer)
## Now
layer = tl.layers.BatchNormLayer(prev_layer = layer)
layer = tl.layers.PReluLayer(prev_layer= layer)
Commit introduced this change: b2e6ccc
Why the API was changed ? As you may guess, just this change lead to many projects raising errors and needing to be updated. We struggle to have tutorials and examples around with TL and this change is not helping with backward compatibility.
2. DeConv2D API Change
## Before
tl.layers.DeConv2d(layer=layer, n_out_channel = 16)
## Now
tl.layers.DeConv2d(layer=layer, n_filter = 16)
Here we have two problems:
- This Layer has now an inconsistent API with the rest of the TL library (this layer use layer instead of prev_layer).
- Again, no deprecation warning with the changes from n_out_channel to n_filter which may immediately make most GANs/AEs not working without a fix.
3. Reuse Variable Scope
You have correctly mentioned a deprecation warning, however it would be better to mention an appropriate fix and not just say "it's deprecated, deal with it now !"
I give you an example:
with tf.variable_scope("my_scope", reuse=reuse) as scope:
# tl.layers.set_name_reuse(reuse) # deprecated
if reuse:
scope.reuse_variables()
Quite easy to add inside the deprecation warning and now it provides a simple solution to fix the issue.
4. No mention in the Changelog of an API change of the ReshapeLayer
## Before
layer = tl.layers.ReshapeLayer(
layer,
shape = [-1, 256, 256, 3]
)
## Now
layer = tl.layers.ReshapeLayer(
layer,
shape = (-1, 256, 256, 3) # Must use a tuple, a list is not accepted anymore
)
Conclusion:
More globally, ALL the changes mentioned above are not explained in the Changelog, and clearly does not help to keep the projects updated. I really love this library, however if we would like to make more people relying on this library we need to be more systematic and serious about the evolutions and changes we bring to the API.
I will try to submit a PR to propose a solution to the problems mentioned above.
Once again, thank you a million times for the work, I really love this library :D
If I ever happened to have missed a discussion or a Changelog mentioning some of the problems raised here, please accept my sincere apologies.
Have great day my friends,
Jonathan