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

Directly set the adjoint of the LHS to 0 in assignments #997

Merged
merged 1 commit into from
Jul 21, 2024

Conversation

PetroZarytskyi
Copy link
Collaborator

Currently, when we set the LHS of an assignment to 0 in the reverse mode, we do this by

float _r_d1 = *_d_y;
*_d_y -= _r_d1;

This PR simplifies this process to

float _r_d1 = *_d_y;
*_d_y = 0;

// Add the statement `dl -= oldValue;`
addToCurrentBlock(BuildOp(BO_SubAssign, AssignedDiff, oldValue),
direction::reverse);
if (!isPointerOp) {
Copy link
Owner

Choose a reason for hiding this comment

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

Out of curiosity, which are the pointer operations in which we should the old strategy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't understand why pointers didn't work at first. The thing here is that for pointers, oldValue is nullptr and BuildOp(...) returns nullptr as well. Because of that, addToCurrentBlock(...) doesn't add anything to the block. So this line didn't do anything for pointers. But the new one does so I had to add a condition.

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link

codecov bot commented Jul 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.92%. Comparing base (7b71e25) to head (fd60dda).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #997   +/-   ##
=======================================
  Coverage   93.92%   93.92%           
=======================================
  Files          55       55           
  Lines        8034     8038    +4     
=======================================
+ Hits         7546     7550    +4     
  Misses        488      488           
Files Coverage Δ
lib/Differentiator/ReverseModeVisitor.cpp 97.29% <100.00%> (+<0.01%) ⬆️
Files Coverage Δ
lib/Differentiator/ReverseModeVisitor.cpp 97.29% <100.00%> (+<0.01%) ⬆️

Copy link
Owner

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

LGTM!

@vgvassilev vgvassilev merged commit f4dcf5c into vgvassilev:master Jul 21, 2024
89 checks passed
@PetroZarytskyi PetroZarytskyi deleted the set-zero branch July 21, 2024 19:25
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