Repository navigation
Refactor SystemSolver - #477
Merged
Merged
Conversation
pelesh
added
enhancement
Collaborator
|
Looks good, it's nice to see smart pointers used in the class. |
shakedregev
reviewed
Oct 5, 2026
shakedregev
left a comment
shakedregev
left a comment
Collaborator
There was a problem hiding this comment.
I still need to test this. On general it looks good. We may want to switch out the quiet defaulting as I flagged in one place.
Comment on lines
+1089
to
+1094
| if (ir_method_ != "none") | ||
| { | ||
| out::warning() << "Incorrect input: iterative refinement cannot be enabled " | ||
| << "together with iterative solve method '" << solve_method_ | ||
| << "'. Setting refinement method to 'none' ...\n"; | ||
| ir_method_ = "none"; |
Collaborator
There was a problem hiding this comment.
I think I prefer to throw an error here. We cannot know if the user intended to use an iterative solver without refinement, or wanted to use a direct solver with refinement. Same applies to other places in the code.
Co-authored-by: Shaked Regev <35384901+shakedregev@users.noreply.github.com>
shakedregev
approved these changes
Oct 5, 2026
shakedregev
left a comment
shakedregev
left a comment
Collaborator
There was a problem hiding this comment.
Tests pass. Leaving it up to you if we want to make the defaulting fixes this time around or in another PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Refactor
SystemSolverclass to check configuration conflicts.Proposed changes
Add helper functions to check and fix conflicts in configuration options and to reduce code duplication.
Checklist
make testandmake test_installper testing instructions). Code tested on./examples/<your_example>.exe -hto get instructions how to run examples). Code tested on:-Wall -Wpedantic -Wconversion -Wextra.Further comments
The diff in
SystemSolver.cppis pretty big, I suggest reviewing as new file, not the differences with respect to earlier version.