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

Add try_forward and try_forward_mut to Module and ModuleMut. #482

Merged
merged 4 commits into from
Feb 24, 2023

Conversation

nkoppel
Copy link
Contributor

@nkoppel nkoppel commented Feb 24, 2023

Closes #323. I've tried my best to convert every operation in implementations of try_forward and try_forward_mut to their try_ counterpart, but it's possible that I have missed some.

Copy link
Owner

@coreylowman coreylowman left a comment

Choose a reason for hiding this comment

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

This looks good to me, nice work! Re the try_broadcast, I wonder if we should remove the fallible version of that (and with permute) since we know they won't allocate new memory. This would make them inconsistent with the other tensor ops though...

src/nn/batchnorm2d.rs Outdated Show resolved Hide resolved
Comment on lines -303 to +329
fn test_batchform2d_3d_repeated_forward_mut() {
fn test_batchnorm2d_3d_repeated_forward_mut() {
Copy link
Owner

Choose a reason for hiding this comment

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

🤣

nkoppel and others added 2 commits February 24, 2023 14:09
Co-authored-by: Corey Lowman <coreylowman@users.noreply.github.com>
@nkoppel
Copy link
Contributor Author

nkoppel commented Feb 24, 2023

In my opinion, we should keep the fallible versions of broadcast and permute, because users shouldn't be exposed to the fact that broadcast and permute can't error when writing fallible code.

@nkoppel nkoppel mentioned this pull request Feb 24, 2023
@coreylowman coreylowman merged commit d351190 into coreylowman:main Feb 24, 2023
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.

Fallible Module & ModuleMut
2 participants