-
Notifications
You must be signed in to change notification settings - Fork 87
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
Add Convergence history #1517
Add Convergence history #1517
Conversation
12ff693
to
6b42557
Compare
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 6 New issues |
6b42557
to
f40587d
Compare
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.
LGTM!
I am not the biggest fan of having an std::vector<std::unique_ptr<LinOp>>
for the residual nor history since it is a bit unintuitive to read from. I would prefer having for example an array<ValueType>
of the norms, for multiple right hand sides this could be strided and have e.g. the last norm as padding for the earlier converged cases.
I'm wondering, instead of modes, should we have distinct classes for the three cases? Their use cases seem pretty distinct |
@upsj IMO, all cases still fit the |
The three modes have three different use cases to me, as well as different interface requirements:
That and the question of the representation that Fritz raised (should the output be stored in a compact format as a single matrix, should we allocate a fixed size and truncate the output to the latest residuals only?) seem important to discuss. |
I'm a bit hesitant to use different classes, because they are subtyping each other. In your example, 3. implements 2. and 1., 2. implements 1., so this would lead to the question of class hierarchy. I don't want to do that here, since I think it would complicate things too much, so I chose the single parameter to switch the behavior. Regarding the compact or segmented storage, I don't see what benefit a full matrix would bring. I would also prefer to not store it as an As @upsj the additional modes are mainly targeted for debugging purposes, and that's also why they were requested. |
I will close this PR as I think the functionality from #1620 is equivalent to this. |
This PR enables the
Convergence
logger to also keep a history of the residual vectors, residual norms, and implicit squared residual norms. There are three modes,