-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add TPU and evaluation saving for GLUE finetuning #234
base: master
Are you sure you want to change the base?
Add TPU and evaluation saving for GLUE finetuning #234
Conversation
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). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
examples/bert/bert_finetune_glue.py
Outdated
@@ -161,12 +179,14 @@ def __init__( | |||
kernel_initializer=initializer, | |||
name="logits", | |||
) | |||
self._drop_out = tf.keras.layers.Dropout(dropout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_dropout_layer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
examples/bert/bert_finetune_glue.py
Outdated
model = keras.models.load_model(FLAGS.saved_model_input, compile=False) | ||
# model = keras.models.load_model(FLAGS.saved_model_input, compile=False) | ||
model = keras.models.load_model( | ||
"gs://chenmoney-testing-east/" + FLAGS.saved_model_input, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs an update!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
@@ -270,6 +306,58 @@ def preprocess_data(inputs, labels): | |||
f"The best hyperparameters found are:\nLearning Rate: {best_hp['lr']}" | |||
) | |||
|
|||
if FLAGS.saved_evaluations_output: | |||
filenames = { | |||
"cola": "CoLA.tsv", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these the exact filename needed for GLUE submission?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, they require the exact name to match
examples/bert/bert_finetune_glue.py
Outdated
@@ -53,6 +53,12 @@ | |||
"The directory to save the finetuned model.", | |||
) | |||
|
|||
flags.DEFINE_string( | |||
"saved_evaluations_output", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe something a little more directly named here? tsv_prediction_output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
with tf.io.gfile.GFile(filename, "w") as f: | ||
# Write the required headline for GLUE. | ||
f.write("index\tprediction\n") | ||
for i in range(test_ds.cardinality()): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like we should be able to do this with a more readable tf.data loop here. for idx, x in enumerate(dataset):
works directly, let's avoid calling iter
and range
and next
ourselves
) | ||
|
||
@tf.function | ||
def eval_step(iterator): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just the actual keras.evaluate here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the predicted label to write to the file. evaluate
only returns metrics?
d1abb4a
to
b0dd210
Compare
examples/bert/bert_finetune_glue.py
Outdated
@@ -161,12 +173,14 @@ def __init__( | |||
kernel_initializer=initializer, | |||
name="logits", | |||
) | |||
self._drop_out_layer = tf.keras.layers.Dropout(dropout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop_out -> dropout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
examples/bert/bert_finetune_glue.py
Outdated
@@ -178,6 +192,7 @@ def __init__(self, model_config): | |||
|
|||
def build(self, hp): | |||
model = keras.models.load_model(FLAGS.saved_model_input, compile=False) | |||
model = model.bert_model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will work right? this seems to be implying we save the whole model in bert_train.py, which we do not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
examples/bert/bert_finetune_glue.py
Outdated
|
||
labelname = labelnames.get(FLAGS.task_name) | ||
with tf.io.gfile.GFile(filename, "w") as f: | ||
# Write the required headline for GLUE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use the builtin csv with a different delimiter to write tsvs
https://docs.python.org/3/library/csv.html
https://stackoverflow.com/q/29895602
We should prefer that over doing this writing outself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
No description provided.