Skip to content
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

docs: various fixes for tf.keras docs. #1119

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

neilconway
Copy link
Contributor

@neilconway neilconway commented Aug 19, 2020

  • Clarify docs on epoch boundaries
  • Fix epoch size in Fashion MNIST example
  • Make wrapper requirements more clear
  • Better linking to API reference.

Description

Test Plan

Commentary (optional)

Copy link
Contributor

@stoksc stoksc left a comment

Choose a reason for hiding this comment

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

lgtm 👍 keras docs look better already.

docs/tutorials/tf-mnist-tutorial.txt Outdated Show resolved Hide resolved
@@ -9,8 +9,7 @@ hyperparameters:
dropout1: 0.25
dropout2: 0.5
resources:
# Use 16 GPUs to train the model.
slots_per_trial: 16
slots_per_trial: 8
Copy link
Contributor

@stoksc stoksc Aug 19, 2020

Choose a reason for hiding this comment

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

What's the reason for this change? Just curious, seems reasonable because this is what the parallel tests run anyways (test_pytorch_parallel).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The quick start tutorial references this file but claims that slots_per_trial is 8. Seems like a reasonable default value anyway...

@@ -713,9 +732,12 @@ def keras_callbacks(self) -> List[tf.keras.callbacks.Callback]:
Determined training behavior.

.. note::
If a callback is supplied that has implemented `keras.callbacks.Callback.on_epoch_end
If you specify a Keras callback that uses the `on_epoch_begin
<https://www.tensorflow.org/api_docs/python/tf/keras/callbacks/Callback#on_epoch_begin>`__
Copy link
Contributor

Choose a reason for hiding this comment

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

ah, good catch adding on_epoch_begin, too.

* Clarify docs on epoch boundaries
* Fix epoch size in Fashion MNIST example
* Make wrapper requirements more clear
* Better linking to API reference.
@neilconway neilconway merged commit a686a37 into determined-ai:master Aug 19, 2020
@dannysauer dannysauer added this to the 0.13.0 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants