Skip to content
This repository was archived by the owner on Apr 10, 2024. It is now read-only.

Conversation

arvind
Copy link
Contributor

@arvind arvind commented Mar 23, 2018

No description provided.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). 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 (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers
  • Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC 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.
  • The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
  • The email used to register you as an authorized contributor must also be attached to your GitHub account.

@coveralls
Copy link

coveralls commented Mar 23, 2018

Pull Request Test Coverage Report for Build 84

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 61.934%

Totals Coverage Status
Change from base Build 79: 0.0%
Covered Lines: 820
Relevant Lines: 1324

💛 - Coveralls

@googlebot
Copy link

CLAs look good, thanks!

@arvind arvind requested a review from ludwigschubert March 23, 2018 19:52
@ludwigschubert
Copy link
Contributor

This was implemented in #59.
Sorry we couldn't merge this PR for that—the global_step tensor also needed initialization.

@arvind
Copy link
Contributor Author

arvind commented May 29, 2018

Apologies for not documenting the motivation for this PR, but I don't think #59 fixes the issue that this PR was trying to tackle: global_step is not available in the T function passed to the objectives functions.

Thus, even after #59, the following custom decay objective fails:

@objectives.wrap_objective
def decay(n_steps):
  def inner(T):
    return 1 - (T('global_step') / n_steps)
  return inner

with the following error:

KeyError: "The name 'import/global_step:0' refers to a Tensor which does not exist. The operation, 'import/global_step', does not exist in the graph."

Is there an alternate way to write the decay objective?

@ludwigschubert
Copy link
Contributor

As discussed in person with @arvind:

  • Current workaround is to directly use tf.train.get_or_create_global_step(). Objective functions get evaluated within a scope that has the default session and graph available.
  • We should rethink T in general, especially the special cases such as input. I think its plausible we don't need T at all, but should probably discuss with everyone first. :-)

@colah colah deleted the global-step branch August 8, 2018 21:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants