search

microsoft / tf2-gnn

TensorFlow 2 library implementing Graph Neural Networks
MIT License
369 stars 73 forks source link

Fix name scopes to avoid unique counters which keep changing #10

Closed mmjb closed 4 years ago

mmjb commented 4 years ago

This keeps breaking loading of model weights by name, which we need for scenarios in which we change the model layout.

The downside of this PR is that it breaks existing models (as in they can't be loaded anymore) - should this trigger a major version number bump?

jackson-flux commented 4 years ago

The changes look good to me.

For the versioning question, we could get some backwards compatibility in this by doing some softer string matching on the set of unused and uninitialised weights, comparing sizes, and loading only then. Would need a big warning on that though.

And the difficulty there is that a lot of the differences would a missing or extra '_{int}' substring, which could just signify a different number of layers in the model.

As the changes are now, I'd say yes to a major version bump. If we can work out a not-horrible way of differentiating between '_{int}' substrings that are caused by layer indices and those caused by keras, then I'd say we go for that and a minor version bump.

mmjb commented 4 years ago

I don't think we can create a good translation scheme - the core problem was that the names were somewhat unstable, leading to these problems in the first place, so translation would be likely to not work reliably. I guess I'll bump the major then...