gan
gan copied to clipboard
Update losses module
This is in reference to issue #24. I was assigned to make the Tutorial Notebook compatible with Keras instead of the Estimator API.
I've already made a new input batch generator and Model Architectures using tfv2 functionalities. But I noticed that the losses module also uses certain tfv1 functionalities. For example, the losses_impl.py
file uses tf.compat.v1.name_scope
instead of tf.name_scope
. I'd like to update the losses module to be tfv2 compatible.
I'm willing to contribute. Please assign this to me.
Hi Saurav,
Thanks for contributing!
Pretty much this whole library is designed around TF1 constructs and in order to get it working with Keras and proper TF2 it probably needs a well thought out wholistic approach. We're also not ready to drop TF1 support, so we either need to have 2 versions of the code or else a way to keep things backwards compatible. The GANModel data structure, which is sort of the center of the whole library relies heavily on variable_scopes, which don't exist in TF2, so that might be the first thing to figure out, and then figure out how to adapt everything else from there.
Would you be interested in coming up with a plan of that sort, and then maybe a proof-of-concept?
The particular issue you point out here could be just one step of the larger plan.
Hey @aaronsarna, that's a great suggestion. I was looking into the variable_scopes
as well while attempting to redefine the wasserstein_loss
function. I'll get started on building up a plan for converting v1.get_variable
to tf.Variable
and similarly to convert v1.variable_scope
to other keras objects like layers/models/module
@aaronsarna do you want me to use a template for the Proof-of-concept?
I think just showing a basic mnist or cifar10 GAN trained with TF2 and Keras is sufficient.
The GANModel class uses the collections module for creating tuple subclasses with named fields. But as per the docs page, this module has been "Deprecated since version 3.3, will be removed in version 3.10". Would you advise me to use the collections.abc module instead or did you have another module in mind?
I don't think there's any need to stick with collections.namedtuple. To some extent the right answer will depend on the overall design. It may just be a class, or you could look at using attr or dataclasses. Whatever makes the most sense given the overall design.