🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/86183

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures, 1 Pending

As of commit dc2140d:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

The committers listed above are authorized under a signed CLA.

• ✅ login: emcastillo (c107aa7, 339bdee, c19e5fc)

Maybe worth introducing an idiomatic utility clone_if(flag) lambda (clone_if = lambda tensor, flag: tensor.clone() if flag else tensor)? this seems a repeated idiom across optimizers, given keeping inplace ops (even if copied inside all optimizers) - although maybe better keep this verbose with if statements as it is now...

separated all the PRs!

Maybe worth introducing an idiomatic utility clone_if(flag) lambda (clone_if = lambda tensor, flag: tensor.clone() if flag else tensor)? this seems a repeated idiom across optimizers, given keeping inplace ops (even if copied inside all optimizers) - although maybe better keep this verbose with if statements as it is now...

Any thoughts about clone_if idiom? Or are explicit different if-statement paths better?

Any thoughts about clone_if idiom? Or are explicit different if-statement paths better?

I'm not sure? t = t.clone() if flag or t seems pretty clear and t = t.clone_if(flag) is not necessarily more readable?
Also we already have too many methods, I'm not sure we want simple helpers like this added.

@pytorchbot merge

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Merge failed

Reason: The following mandatory check(s) failed (Rule superuser):

• pull

Dig deeper by viewing the failures on hud

Any thoughts about clone_if idiom? Or are explicit different if-statement paths better?

I'm not sure? t = t.clone() if flag or t seems pretty clear and t = t.clone_if(flag) is not necessarily more readable? Also we already have too many methods, I'm not sure we want simple helpers like this added.

well, i meant that instead of if differentiable: if-else statements all over the optimizers that only change add_ to add etc. we would have a clone_if helper lambda shared across these optimizers and then the code would be clone_if(tensor, differentiable).add_ and keep only one code version. Since the lambda is trivial, it could be copied across the optimizer impls if wanting to keep their codes more independent

t = t.clone() if flag or t seems pretty clear and t = t.clone_if(flag) is not necessarily more readable?
Yeah, agree, but I mean somehow using this as idioum, for that a lambda clone_if may be better

Then instead of
if differentiable:
tensor = tensor.add(eps)
else:
tensor = tensor.add_(eps)

could be always written:
tensor = clone_if(tensor, differentiable).add_(eps) - although maybe this is less efficient?

Also, is there a functorch higher-order function wrappers that can act somehow torch.add(tensor, eps, inplace = not differentiable) to choose between torch.add and torch.add_?

although maybe this is less efficient?

Yes, we go twice over the memory, once to copy it and once to add.

Also, is there a functorch higher-order function wrappers that can act somehow torch.add(tensor, eps, inplace = not differentiable) to choose between torch.add and torch.add_?

Maybe? That would be a lot of work to add that to all APIs :p

actually, maybe not so many, not that many ops support proper inplace...

Also functorch really doesn't like inplace in general ;) So I don't think they will be happy with adding inplace kwarg haha
cc @zou3519

Well, maybe not in functorch :) in core

You can skip the dynamo errors.
The other issues seem relevant right?

@pytorchbot merge

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team