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

Introduce move semantics to adouble, clean-up tape types, clean-up include files #96

Open
wants to merge 85 commits into
base: master
Choose a base branch
from

Conversation

TimSiebert1
Copy link
Collaborator

This PR:

  • implements move semantics for the tape-based derivative type adouble as proposed in Move sematics for adouble #94;
  • cleans-up the tape-based ad-type system;
  • cleans-up the includes.

The main idea is that adubs are completely replaced by proper treatment of r-value references. This results in the updated adouble class without the need for an inheritance structure. Thus, badouble and adub are removed.

Prior to the proposed changes, each arithmetic operation generates a new tape-location regardless of whether the input is a temporary (r-value reference) or not. This potentially lead to a significant number of "wasted" locations.

In the proposed changes and for most operations the arithmetic operation does creating a new adouble if the input is an r-value reference. To this end the operations are overloaded for r-value references, there is a move constructor and a move assignment.

As a result, an expression like 0.5 * (x + u/x), which would create in the current implementation three adub (basically adoubles) with new locations, the new version would only create a single new location and move it between the operations.

Please note that I introduce a lot of code-duplications when defining the operations for r-value references. They should be definitely removed at some point and are an item on my list 😃

…rary adouble object. However, this can be directly achieve by by C++'s move semantics. On the other hand we start replacing the plain size_t location with a more type-safe struct "tape_location". Finally, the inheritance structure (badouble, adouble, adub) is removed, because its not needed with the changes explained above.
…cs, shift function declarations to adouble class
…erf and erfc to new class structure and move semantics
…quad to new type structure and move semantics
…cs out of class and leverage getter of adouble and pdouble
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 85.60061% with 380 lines in your changes missing coverage. Please review.

Project coverage is 72.73%. Comparing base (29d60c0) to head (530e7e8).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
ADOL-C/src/advector.cpp 0.00% 191 Missing ⚠️
ADOL-C/src/pdouble.cpp 39.45% 112 Missing ⚠️
ADOL-C/src/externfcts.cpp 51.21% 40 Missing ⚠️
ADOL-C/src/fixpoint.cpp 81.13% 10 Missing ⚠️
ADOL-C/src/checkpointing.cpp 90.80% 8 Missing ⚠️
ADOL-C/include/adolc/advector.h 0.00% 6 Missing ⚠️
ADOL-C/src/externfcts2.cpp 0.00% 6 Missing ⚠️
ADOL-C/src/storemanager.cpp 0.00% 3 Missing ⚠️
ADOL-C/include/adolc/adtb_types.h 96.15% 2 Missing ⚠️
ADOL-C/src/taping.cpp 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
+ Coverage   68.49%   72.73%   +4.24%     
==========================================
  Files          53       57       +4     
  Lines       28596    30934    +2338     
  Branches     1861     1988     +127     
==========================================
+ Hits        19586    22501    +2915     
+ Misses       9010     8433     -577     
Flag Coverage Δ
unittests 72.73% <85.60%> (+4.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@osander1
Copy link
Contributor

That's a lot of stuff for a single pull request. If possible, splitting it into several parts would make it easier to review.

@TimSiebert1
Copy link
Collaborator Author

Hmm, that would probably be a lot of work... But if you want to check each PR carefully, I could do that. Otherwise, I could explain the changes in more detail in a meeting.

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