Now able to use and create custom tensorflow heads, embedders, and middleware.

[ryanpeach]

Ref #134

I modified the following classes:

• HeadParameters
• MiddlewareParameters
• InputEmbedderParameters

Adding a path property (or function as I mention in challenges).

Then I modified:

• GeneralTensorFlowNetwork.get_input_embedder
• GeneralTensorFlowNetwork.get_middleware
• GeneralTensorFlowNetwork.get_output_head

To use these paths instead of their own local's.

I moved a local dictionary inside GeneralTensorFlowNetwork.get_input_embedder called mod_names to embedder_parameters.MOD_NAMES so that it's more accessible.

Challenges

• InputEmbedderParameters.path can not be a property like the rest. You can call it with emb_type and the path will be created. But that's different than how most path's are made.

Pytest

I ran pytest locally and do not see any dramatic changes in the number of passing tests.

[aipg-hub]

Can one of the admins verify this patch?

[gal-leibovich]

LGTM.
Currently the golden tests are not running with the CI. Could you please run tests/test_golden.py locally to make sure that golden tests are passing?

@scttl CI is failing with Unable to locate credentials. You can configure credentials by running "aws configure". Exited with code 255. Could you please take a look?

@safrooze Please take a look. Note that setting a user defined Embedder/MW/Head is currently not supported with mxnet (and this PR will not fix that).

[gal-leibovich]

LGTM. Need to have tests passing, and rebase.

[ryanpeach]

I couldn't find a similar area of code for mxnet to fix. If it exists I'll fix that too. Also the CI does not appear to be passing master at the moment. It would be a bit difficult to replicate the environment on my system, but for the ones that dont have extra dependencies they pass last time I checked. Will this be part of a versioned release?

…

________________________________ From: Gal Leibovich <notifications@github.com> Sent: Monday, December 3, 2018 7:28:01 AM To: NervanaSystems/coach Cc: Ryan Peach; Author Subject: Re: [NervanaSystems/coach] Now able to use and create custom tensorflow heads, embedders, and middleware. (#135) @galleibo-intel approved this pull request. LGTM. Need to have tests passing, and rebase. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#135 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AOJrye1aZvXMN4J0pgrvpGutr8s_wFsiks5u1RjRgaJpZM4Y8ksM>.

[scttl]

Yep the CI wasn't initially set to run on forked builds (just changed and kicked off a build for this and other open PRs).

[ryanpeach]

I could also run via the Amazon released docker container if that has all the dependencies. I heard about this project and am using it via the new Sagemaker RL.

[ryanpeach]

Thanks for fixing the CI much appreciated.

[ryanpeach]

What is your opinion on InputEmbedderParameters.path?

[gal-leibovich]

Although it differs from the solution used on other modules, I think that having it as method makes sense in this InputEmbedder case.

If you could rebase your branch, we can merge it to master.