Skip to content
This repository was archived by the owner on Jul 7, 2023. It is now read-only.

Conversation

@noe
Copy link
Contributor

@noe noe commented Jul 17, 2017

As described in issue #134 , it is now possible to add new problems by subclassing Problem and registering it, but there is no analogous mechanism to parameter --t2t_usr_dir from t2t-trainer which allows loading user-provided code. This PR simply consists in factoring out the functionality of --t2t_usr_dir out of t2t-trainer to a common place, and use it from both t2t-trainer and t2t-datagen.

@noe noe changed the title Support --usr_dir also in t2t-datagen Support --usr_dir also in t2t-datagen Jul 17, 2017
@noe noe changed the title Support --usr_dir also in t2t-datagen Support --t2t_usr_dir also in t2t-datagen Jul 17, 2017
Copy link
Contributor

@rsepassi rsepassi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! One small edit requested, but otherwise looks good!

import importlib
import tensorflow as tf

flags = tf.flags
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you define the flag in the t2t-trainer as well as the t2t-datagen and have import_usr_dir take a usr_dir as an argument. yes, it'll be duplicated, but generally we try to keep flags in the main "binary" file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the PR with the requested changes.

Copy link
Contributor

@rsepassi rsepassi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@rsepassi rsepassi merged commit 0d250b3 into tensorflow:master Jul 18, 2017
rsepassi pushed a commit to rsepassi/tensor2tensor that referenced this pull request Jul 19, 2017
PiperOrigin-RevId: 162389811
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.

2 participants