Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Feature/pre trained embeddings #269

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Feature/pre trained embeddings #269

wants to merge 12 commits into from

Conversation

pratapbhanu
Copy link

This PR includes following changes:

  • support for using pre-trained embeddings as source embeddings.
  • configuration option for fine-tuning these embeddings.
  • corresponding tests

@darrengarvey
Update imports to support TensorFlow v1.2.
a31c1fd
@darrengarvey
Fix typo in hooks_test.py.
8053094
@darrengarvey
Update hooks_test.py to work with TensorFlow 1.2.
4448976
@darrengarvey
Fix hooks_test.py on Python 3.
f85d026 
Fixes the following error when running the tests on Python 3.

    Traceback (most recent call last):
      File "seq2seq/seq2seq/test/hooks_test.py", line
      55, in test_begin
          self.assertEqual(file_contents.decode(), ...)
          AttributeError: 'str' object has no attribute 'decode'
@darrengarvey
Another go at fixing hooks_test.py.
83d592f 
distutils.version isn't available on the CI versions, so change the test
to not rely explicitly on the version of TensorFlow installed.
@darrengarvey
Tell pylint about the tf contextmanager.
ffb1505 
Use of `tf.name_scope` and `tf.variable_scope` cause pylint errors on TF1.2
due to pylint not fully understanding the `tf_contextlib.contextmanager`
decorators.
@darrengarvey
Work around no-name-in-module pylint errors.
0c20ee4 
Following some changes in TF to the `LazyLoader` [1], pylint complains
about not being able to find some imports under `tf.contrib`. This looks
like a pylint issue, emitting errors like:

    ************* Module seq2seq.encoders.rnn_encoder
    E: 24, 0: No name 'rnn' in module 'LazyLoader' (no-name-in-module)
    ************* Module seq2seq.data.input_pipeline
    E: 32, 0: No name 'slim' in module 'LazyLoader' (no-name-in-module)

[1] tensorflow/tensorflow@95c5d7e
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@pratapbhanu
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@pratapbhanu
Copy link
Author

Ok, Failed CircleCI tests are not specific to this PR, pylint tests for almost all modules are failing:

Context manager 'generator' doesn't implement __enter__ and __exit__. (not-context-manager)

@darrengarvey
Copy link

darrengarvey commented Jul 9, 2017 •
edited
Loading

PR #254 should fix the various TF 1.2 / python versioning problems.

@pratapbhanu pratapbhanu force-pushed the feature/pre-trained-embeddings branch from 9f2b5a0 to 78c396a Compare July 10, 2017 08:38
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@pratapbhanu
Copy link
Author

pratapbhanu commented Jul 10, 2017 •
edited
Loading

Rebased on #254 from @darrengarvey

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pratapbhanu @googlebot @darrengarvey