Model specific custom_objects for CremaModel

[beasteers]

In CremaModel._instantiate, you use layers defined in layers.py as available custom objects when loading a model. Should we make it easier to add arbitrary custom objects?

Two quick ideas that I had:

class SomeModel(CremaModel):
    # This would be defined in the subclass
    CUSTOM_OBJECTS = {
        'asdf': asdf, 'gfds': gfds
    }

    # This would be defined in base.CremaModel
    def _instantiate(self, ...):
        ...
        custom_objects = {k: layers.__dict__[k] for k in layers.__all__}

        # THIS:
        custom_objects.update(self.CUSTOM_OBJECTS)

        # OR:

        custom_objects_file = resource_filename(
            __name__, os.path.join(rsc, 'custom_objects.pkl')

        if os.path.isfile(custom_objects_file):
            with open(custom_objects_file, 'rb') as f:
                custom_objects.update(pickle.load(custom_objects_file))

        ##

        self.model = model_from_config(spec, custom_objects=custom_objects)
        ...

we could use one or the other, or both. I'm partial to the second one tho because it doesn't require subclassing.

[bmcfee]

Should we make it easier to add arbitrary custom objects?

It's a good question. Really this all comes down to limitations of keras (and, to some extent, pickle/json). My basic position is that we should use custom objects sparingly, and try to keep them as general as possible to maximize reuse -- squeezelayer is a pretty good example of that I think, so would autopool. For that reason, I prefer coding them directly into the layers module, but i'm open to counter-arguments. Did you have something specific in mind?

[beasteers]

I guess I'm not sure what the idea behind Crema is. Is it supposed to be scaffolding that other people import and build models on top of? Or is it meant to have all of the models self-contained and integrated into the master branch?

If it's the latter, then I understand why you'd have things written into the layers.py file. If it's the former, then it makes it a bit cumbersome to use (having to monkeypatch any custom objects into crema.layers).

For autopool, are you thinking that you'd copy in the source code into layers.py? Or just include it as a dependency and import+export? (Just because it sounds like models are supposed to be dependency free, but in this case, that would mean having an alternate implementation)

For SqueezeLayer, I'm pretty sure keras.backend has that built in now btw (backend functions seem to double as layers): https://www.tensorflow.org/api_docs/python/tf/keras/backend/squeeze

Really this all comes down to limitations of keras (and, to some extent, pickle/json)

Honestly, I'm not sure why model.save can't traverse the model definition and pickle any custom objects and save it with the definition/weights. Seems like that would be super convenient, but I guess that's a question for the keras team.

[bmcfee]

Is it supposed to be scaffolding that other people import and build models on top of? Or is it meant to have all of the models self-contained and integrated into the master branch?

Primarily the latter.

If it's the former, then it makes it a bit cumbersome to use (having to monkeypatch any custom objects into crema.layers).

I suppose we could put some helpers in to make the monkeypatching less problematic, but then i'd worry that some models wouldn't be able to import without run-time patching in advance, and that seems bad.

[beasteers]

i'd worry that some models wouldn't be able to import without run-time patching in advance, and that seems bad.

That's why I think having them pickled would be the best way. Then whoever designed the model has to make sure that any custom objects are importable/available before calling Model._instantiate

It also means that if a model isn't being used, then dependencies can be considered optional which can make things lighter weight