vgvassilev / clad

clad -- automatic differentiation for C/C++
GNU Lesser General Public License v3.0
266 stars 113 forks source link

Emit clad::pop after clad::back when in RMV::VisitBinaryOperator #936

Closed PetroZarytskyi closed 3 weeks ago

PetroZarytskyi commented 3 weeks ago

After #904, operands in multiplication and division inside loops were not correctly stored anymore. The pop expression was emitted before the value was used:

// forward sweep (inside a loop)
    ...
    clad::push(_t3, arr[0]);
    out += clad::back(_t3) * params[0];
    ...

// reverse sweep (inside a loop)
    ...
    clad::pop(_t3);    <- the value is popped before it's used
    _d_params[0] += clad::back(_t3) * _r_d0;
    ...

This is because in RMV::VisitBinaryOperator the order of statements in the corresponding reverse block is not reversed (while in all other use cases of GlobalStoreAndRef, the order of statements is reversed). This PR uses a simple workaround to fix this bug. Fixes #927

github-actions[bot] commented 3 weeks ago

clang-tidy review says "All clean, LGTM! :+1:"

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.76%. Comparing base (db0a949) to head (4220f19).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/vgvassilev/clad/pull/936/graphs/tree.svg?width=650&height=150&src=pr&token=9f6Q4em8hE&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev)](https://app.codecov.io/gh/vgvassilev/clad/pull/936?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev) ```diff @@ Coverage Diff @@ ## master #936 +/- ## ========================================== + Coverage 93.72% 93.76% +0.03% ========================================== Files 54 54 Lines 7781 7781 ========================================== + Hits 7293 7296 +3 + Misses 488 485 -3 ``` | [Files](https://app.codecov.io/gh/vgvassilev/clad/pull/936?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev) | Coverage Δ | | |---|---|---| | [lib/Differentiator/ReverseModeVisitor.cpp](https://app.codecov.io/gh/vgvassilev/clad/pull/936?src=pr&el=tree&filepath=lib%2FDifferentiator%2FReverseModeVisitor.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev#diff-bGliL0RpZmZlcmVudGlhdG9yL1JldmVyc2VNb2RlVmlzaXRvci5jcHA=) | `97.30% <100.00%> (+0.13%)` | :arrow_up: | | [Files](https://app.codecov.io/gh/vgvassilev/clad/pull/936?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev) | Coverage Δ | | |---|---|---| | [lib/Differentiator/ReverseModeVisitor.cpp](https://app.codecov.io/gh/vgvassilev/clad/pull/936?src=pr&el=tree&filepath=lib%2FDifferentiator%2FReverseModeVisitor.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev#diff-bGliL0RpZmZlcmVudGlhdG9yL1JldmVyc2VNb2RlVmlzaXRvci5jcHA=) | `97.30% <100.00%> (+0.13%)` | :arrow_up: |
github-actions[bot] commented 3 weeks ago

clang-tidy review says "All clean, LGTM! :+1:"