-
Notifications
You must be signed in to change notification settings - Fork 845
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
[WIP] Tag / debug tape recordings #2343
base: develop
Are you sure you want to change the base?
Conversation
…l write restart files during runtime (there is a bug, potentially when clearing the tape).
@@ -744,7 +744,7 @@ | |||
if (kind_recording == RECORDING::SOLUTION_VARIABLES) { | |||
cout << " Objective function : " << ObjFunc; | |||
if (driver_config->GetWrt_AD_Statistics()){ | |||
cout << " (" << ObjFunc_Index << ")\n"; | |||
// cout << " (" << ObjFunc_Index << ")\n"; |
Check notice
Code scanning / CodeQL
Commented-out code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be resolved inside CoDiPack :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, can you document how it would be used?
…zation.hpp (to cover RealReverseTag).
Sure. Added an option so that you can compile the AD build using the tag type ( |
static constexpr bool StoreAsRef = true; | ||
|
||
private: | ||
alignas(Size * sizeof(Scalar)) Scalar x_[N]; | ||
alignas(Size * 32) Scalar x_[N]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you be okay with replacing 32
by a function to find the next power of 2? (Requirement of alignas.) Or could this be done any better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what type is the result not a power of 2?
If we use a fixed number instead of sizeof, arrays of this type may take more space than necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reverse tag type is 24 I think. If sizeof(Scalar)
is already a power of 2 that function should of course not change it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, so the tag doesn't have a float/double part? Then it should be ok to round Size * sizeof(Scalar) to the next power of 2.
Or maybe using Size * alignof(Scalar), that may be more generic and achieve the desired result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it does contain at least a double for the primal value (show must go on). I'll check or ask what the remaining bytes are for besides two int
s for index and tag. But sizeof(RealReverseTag) = 24
.
…code/SU2 into feature_mz_adjoint_for_turbo
…eature_mz_adjoint_for_turbo
…his is a temporary change to avoid false positives when a tag tape is used. They might be unnecessary anyway though.
…te adjoint solver.
…g mode finding :-))
These are WIP commits just in order to make the debug mode already available to e.g. @joshkellyjak :-) |
@@ -515,9 +536,9 @@ | |||
FORCEINLINE void SetPreaccIn(const T& data, const int size) { | |||
if (PreaccActive) { | |||
for (int i = 0; i < size; i++) { | |||
if (IsIdentifierActive(data[i])) { | |||
// if (IsIdentifierActive(data[i])) { |
Check notice
Code scanning / CodeQL
Commented-out code
PreaccHelper.addInput(data[i]); | ||
} | ||
// } |
Check notice
Code scanning / CodeQL
Commented-out code
@@ -527,9 +548,9 @@ | |||
if (!PreaccActive) return; | |||
for (int i = 0; i < size_x; i++) { | |||
for (int j = 0; j < size_y; j++) { | |||
if (IsIdentifierActive(data[i][j])) { | |||
// if (IsIdentifierActive(data[i][j])) { |
Check notice
Code scanning / CodeQL
Commented-out code
PreaccHelper.addInput(data[i][j]); | ||
} | ||
// } |
Check notice
Code scanning / CodeQL
Commented-out code
SetPreaccOut(moreData...); | ||
} | ||
|
||
template <class T> | ||
FORCEINLINE void SetPreaccOut(T&& data, const int size) { | ||
if (PreaccActive) { | ||
for (int i = 0; i < size; i++) { | ||
if (IsIdentifierActive(data[i])) { | ||
// if (IsIdentifierActive(data[i])) { |
Check notice
Code scanning / CodeQL
Commented-out code
PreaccHelper.addOutput(data[i]); | ||
} | ||
// } |
Check notice
Code scanning / CodeQL
Commented-out code
@@ -567,9 +589,9 @@ | |||
if (!PreaccActive) return; | |||
for (int i = 0; i < size_x; i++) { | |||
for (int j = 0; j < size_y; j++) { | |||
if (IsIdentifierActive(data[i][j])) { | |||
// if (IsIdentifierActive(data[i][j])) { |
Check notice
Code scanning / CodeQL
Commented-out code
PreaccHelper.addOutput(data[i][j]); | ||
} | ||
// } |
Check notice
Code scanning / CodeQL
Commented-out code
This reverts commit 683de77. Working on the wrong branch.
… into feature_tag_debug_tape
I'm sorry for the single fix commit, this PR wasn't meant to include all the fixes alongside the debug mode implementation (it was just meant as an example). We can continue to add them here but I'm afraid this PR becomes less readable then. |
I think I have already made this pretty unreadable, everything from my fork keeps pushing here. |
e6fd4aa
to
1169606
Compare
Proposed Changes
Add feature to record a tape for debug purposes (as a tool for AD / discrete adjoint / .. developers), using CoDiPack's
codi::RealReverseTag
.Related Work
Supposed to help finding remaining errors in #2317.
PR Checklist
Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.
pre-commit run --all
to format old commits.