vgvassilev / clad

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

Use connected lists for tape data structure #954

Open rohanjulka19 opened 2 weeks ago

rohanjulka19 commented 2 weeks ago

Currently I have added a basic implementation of tape data structure with connected lists/slabs. It is more or less similar to current tape data strucutre I copied a lot of code just added blocks and only added the functionality which I feel is needed for reverse mode.

Right now NumericalDiff also uses tape so I defined two types one is "tape" which points to the new tape implementation and "old_tape" which points to current tape data strcuture being used.

Tests are running fine for the new implementation.

These are the benchmarks for old_tape and new_tape.

LastTest-new-tape.log LastTest-tape.log

Fixes - #793

vgvassilev commented 4 days ago

@rohanjulka19, do you mind addressing the clang-tidy complaints where relevant?

rohanjulka19 commented 4 days ago

hi, yeah will address these clang-tidy issues, actually i got stuck on the loop condition differentiation issue (once again) https://github.com/vgvassilev/clad/pull/818. But now I am fixing that one, once I push that fix will address these then.

rohanjulka19 commented 1 day ago

@vgvassilev I have fixed most of the suggestion, the above four I don't think needs to be addressed.

vgvassilev commented 1 day ago

@vgvassilev I have fixed most of the suggestion, the above four I don't think needs to be addressed.

The include protector suggestions seem good. Looks like the new tape has a problem with cuda.

codecov[bot] commented 1 day ago

Codecov Report

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

Project coverage is 93.94%. Comparing base (8749404) to head (38fdfb3).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/vgvassilev/clad/pull/954/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/954?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 #954 +/- ## ======================================= Coverage 93.94% 93.94% ======================================= Files 55 55 Lines 7965 7965 ======================================= Hits 7483 7483 Misses 482 482 ``` | [Files](https://app.codecov.io/gh/vgvassilev/clad/pull/954?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev) | Coverage Δ | | |---|---|---| | [include/clad/Differentiator/Differentiator.h](https://app.codecov.io/gh/vgvassilev/clad/pull/954?src=pr&el=tree&filepath=include%2Fclad%2FDifferentiator%2FDifferentiator.h&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev#diff-aW5jbHVkZS9jbGFkL0RpZmZlcmVudGlhdG9yL0RpZmZlcmVudGlhdG9yLmg=) | `83.33% <ø> (ø)` | | | [Files](https://app.codecov.io/gh/vgvassilev/clad/pull/954?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev) | Coverage Δ | | |---|---|---| | [include/clad/Differentiator/Differentiator.h](https://app.codecov.io/gh/vgvassilev/clad/pull/954?src=pr&el=tree&filepath=include%2Fclad%2FDifferentiator%2FDifferentiator.h&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Vassil+Vassilev#diff-aW5jbHVkZS9jbGFkL0RpZmZlcmVudGlhdG9yL0RpZmZlcmVudGlhdG9yLmg=) | `83.33% <ø> (ø)` | |
rohanjulka19 commented 1 day ago

The include protector suggestions seem good. Looks like the new tape has a problem with cuda.

Have changed the include protector to align with the suggestion. The cuda test is passing but I am not sure if it will work when this runs on more than one thread in parallel