Skip to content

Added colab notebook for state params#1932

Merged
copybara-service[bot] merged 8 commits intogoogle:mainfrom
AdityaKane2001:patch-1
Feb 23, 2022
Merged

Added colab notebook for state params#1932
copybara-service[bot] merged 8 commits intogoogle:mainfrom
AdityaKane2001:patch-1

Conversation

@AdityaKane2001
Copy link
Contributor

@AdityaKane2001 AdityaKane2001 commented Feb 22, 2022

Regarding #1910
Added "Open in Colab" button which points to a Colab notebook having executable code of the Managing Parameters and State guide.

Checklist

  • This PR fixes a minor issue (e.g.: typo or small bug) or improves the docs (you can dismiss the other
    checks if that's the case).
  • This change is discussed in a Github issue/
    discussion (please add a
    link).
  • The documentation and docstrings adhere to the
    documentation guidelines.
  • This change includes necessary high-coverage tests.
    (No quality testing = no merge!)

@AdityaKane2001
Copy link
Contributor Author

@marcvanzee

Could you please take a look at this one? TIA

@marcvanzee marcvanzee self-requested a review February 23, 2022 09:30
@marcvanzee
Copy link
Contributor

Thanks a lot @AdityaKane2001!

Would it be possible to also include the Colab itself in this PR, and add it to flax/docs/howtos/? Then we will be able to update this if we need to change anything.

I also found that the Colab doesn't work, you need to change to dummy input to:

dummy_input = jnp.ones((32, 5))

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@AdityaKane2001
Copy link
Contributor Author

@marcvanzee

Could you please take a look and let me know it this is what you wanted?

Copy link
Contributor

@marcvanzee marcvanzee left a comment

Choose a reason for hiding this comment

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

Thanks! Just a few small comments.

@@ -1,3 +1,6 @@
.. image:: https://colab.research.google.com/assets/colab-badge.svg
:target: https://colab.research.google.com/gist/AdityaKane2001/cbd09c2fcb86163cb301076cb60ce200/state_params_howto.ipynb
Copy link
Contributor

Choose a reason for hiding this comment

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

Please point to the Colab in this repo

"opt_state = tx.init(params)\n",
"\n",
"for _ in range(num_epochs):\n",
" opt_state, params, state, loss = update_step(\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to print something. maybe print the loss after each epoch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I didn't add that earlier because I thought it would be inconsistent with the docs.

@marcvanzee
Copy link
Contributor

It seems you cannot use %%capture in Colabs (it doesn't work with Sphinx). Could you remove that from the Colab as well? Right now it breaks our build.

@AdityaKane2001
Copy link
Contributor Author

Should I keep #@title or should I remove that as well?

@marcvanzee
Copy link
Contributor

Should I keep #@title or should I remove that as well?

I think you can keep that, since it is in a comment.

Copy link
Contributor

@marcvanzee marcvanzee left a comment

Choose a reason for hiding this comment

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

Looks great, thanks a lot!

@copybara-service copybara-service bot merged commit 13ef3b0 into google:main Feb 23, 2022
@AdityaKane2001
Copy link
Contributor Author

Thanks for the merge!

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.

2 participants