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

Move sematics for adouble #94

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

Conversation

osander1
Copy link
Contributor

@osander1 osander1 commented Jan 3, 2025

The adub class seems to implement what modern C++ calls 'r-values' -- temporary objects that never get a name of their own. If this is true then it should be possible to get rid of adub completely, and replace it by move-semantics for adouble. This MR contains a few first steps into that direction.

Please test and review them thoroughly. I have not understood all the details yet. I have used the patches for a few weeks without problems, but there is one test failure testing extdiff. Note that verifying that the patched code still runs and produces correct results is not enough: One has to check that the patches do not lead to longer tapes (which they shouldn't).

The adub data type is used to only store temporary values,
and once an adub object is assigned to an adouble object
it shall never be used again.  Therefore, assignment of
adub to adouble can be implemented just like move-assignment,
without having to touch the tape at all.

For this to work, the method signature has to change from
(const adub&) to (adub&), because the value that is assigned
is now modified (it is invalidated).
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@TimSiebert1
Copy link
Collaborator

Hi @osander1,

due to architectural changes, the 'adub' could be removed. Until then, I would not review it.
I will present the architecture in the next 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.

3 participants