vgvassilev / clad

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

Add support for && operator #923

Closed rohanjulka19 closed 1 week ago

github-actions[bot] commented 4 weeks ago

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

codecov[bot] commented 4 weeks ago

Codecov Report

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

Project coverage is 93.85%. Comparing base (1267d57) to head (eefcc20).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/vgvassilev/clad/pull/923/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/923?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 #923 +/- ## ========================================== + Coverage 93.83% 93.85% +0.02% ========================================== Files 55 55 Lines 7878 7892 +14 ========================================== + Hits 7392 7407 +15 + Misses 486 485 -1 ``` | [Files](https://app.codecov.io/gh/vgvassilev/clad/pull/923?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/923?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.26% <100.00%> (+0.01%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/vgvassilev/clad/pull/923/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev) | [Files](https://app.codecov.io/gh/vgvassilev/clad/pull/923?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/923?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.26% <100.00%> (+0.01%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/vgvassilev/clad/pull/923/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev)
github-actions[bot] commented 3 weeks ago

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

parth-07 commented 3 weeks ago

Please also improve the commit message. This guide might be helpful for improving the commit message: https://cbea.ms/git-commit/

github-actions[bot] commented 3 weeks ago

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

vgvassilev commented 2 weeks ago

@rohanjulka19, ping. Maybe you can take a look at https://github.com/vgvassilev/clad/blob/master/CONTRIBUTING.md#creating-a-successfull-pull-request address @parth-07's comment.

rohanjulka19 commented 2 weeks ago

https://github.com/vgvassilev/clad/blob/master/CONTRIBUTING.md#creating-a-successfull-pull-request

oh I had already addressed them, follwing the guidelines mentioned in the provided link - https://cbea.ms/git-commit/

Regarding this - https://github.com/vgvassilev/clad/blob/master/CONTRIBUTING.md#creating-a-successfull-pull-request

  1. Currently there is no issue created for this I can link this with Incorrect differentiation of loop conditions in reverse mode #746 issue as this PR was opened as part of solution for this.

  2. I haven't added tests I was hoping if parth is ok with the changes then I will add the tests otherwise it gets tedious to fix them again and again.

From next time will re-request review. I usually don't do that to prevent annoying the reviewer.

vgvassilev commented 2 weeks ago

2. I haven't added tests I was hoping if parth is ok with the changes then I will add the tests otherwise it gets tedious to fix them again and again.

That sounds backwards. We require tests as they help reviewers understand the overall intent of the change.

rohanjulka19 commented 2 weeks ago
  1. I haven't added tests I was hoping if parth is ok with the changes then I will add the tests otherwise it gets tedious to fix them again and again.

That sounds backwards. We require tests as they help reviewers understand the overall intent of the change.

Yeah I guess ok will add tests to this

rohanjulka19 commented 2 weeks ago

Added Validation for Empty if block here - https://github.com/vgvassilev/clad/pull/952

Now in case of

if(a && b ) {}

the diff will be

double _d_cond1;
if(_cond0) { 
_cond1 = b 
}
_cond2 = _cond0 && _cond1;

Since I want the _cond1 expression inside the if block to be differentiated and not ignored that's why had to create a derivative variable of _cond1 and add it to m_Variables map.

github-actions[bot] commented 2 weeks ago

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

vgvassilev commented 2 weeks ago

Can you properly rebase this PR?

github-actions[bot] commented 2 weeks ago

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

github-actions[bot] commented 1 week ago

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