Skip to content

Minor cleanup for optax.projections. #1277

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

carlosgmartin
Copy link
Contributor

@carlosgmartin carlosgmartin commented Apr 24, 2025

Minor cleanup for optax.projections. Includes the following changes:

  1. Change || to \| in docstring math formulas. The latter renders the double bars used in the notation for norms correctly.
  2. Inline unnecessary function calls.
  3. Replace float constants with ints, where possible, to avoid unnecessary conversions to float in some cases when the input is integral.
  4. Remove unnecessary scale is None conditional in projection_simplex.
  5. Remove unnecessary use of jax.lax.cond, which can be expensive, in projection_l2_ball.
  6. Shorten unnecessarily verbose code.

@carlosgmartin carlosgmartin force-pushed the projections_cleanup branch 3 times, most recently from 060eb71 to ce11ca1 Compare April 24, 2025 20:16
Copy link
Collaborator

@vroulet vroulet left a comment

Choose a reason for hiding this comment

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

Thanks, minor comments left, then we can merge.
Did you build the docs to quickly check them too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants