Use enum for SystemSolver preconditioner side

[verma-divyanshu-git]

Description

Use Preconditioner::Side at the CLI boundary for the sysGmres example and functionality test while preserving the typed SystemSolver API.

Closes #439

Proposed changes

• keep SystemSolver::preconditionerSetup(Preconditioner::Side) typed in resolve/SystemSolver.hpp and resolve/SystemSolver.cpp
• parse -p left|right into Preconditioner::Side in examples/sysGmres.cpp and tests/functionality/testSysGmres.cpp, defaulting to RIGHT only when -p is omitted
• return a clear nonzero error for explicit invalid -p values instead of silently falling through to right preconditioning
• add sys_gmres_invalid_preconditioner_side_test with WILL_FAIL TRUE in tests/functionality/CMakeLists.txt
• switch the inherited warning-path log call in tests/functionality/testSysGmres.cpp to ReSolve::io::Logger::misc() so the functionality target builds during warning-enabled CPU validation

Checklist

• All tests pass (make test and make test_install per testing instructions). Code tested on
  • CPU backend
  • CUDA backend
  • HIP backend
• I have manually run the non-experimental examples and verified that residuals are close to machine precision. (In your build directory run: ./examples/<your_example>.exe -h to get instructions how to run examples). Code tested on:
  • CPU backend
  • CUDA backend
  • HIP backend
• Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
• The new code follows Re::Solve style guidelines.
• There are unit tests for the new code.
• The new code is documented.
• The feature branch is rebased with respect to the target branch.
• I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

• make test: 51/51 passed.
• make test_install: consumer ctest 1/1 passed.
• Automated functionality coverage: sys_gmres_invalid_preconditioner_side_test runs sys_rand_gmres_test.exe -p bogus with WILL_FAIL TRUE; explicit invalid -p now prints Preconditioning side 'bogus' not recognized. Use 'left' or 'right'. and exits 1.
• Focused functionality checks on CPU:
  • default sys_rand_gmres_test.exe -x no -i fgmres -g cgs2: residual 2.1137376763106819e-16
  • -p right: residual 2.1137376763106819e-16
  • -p left: residual 2.0607333461083850e-16
• Manual CPU example checks:
  • default omitted -p: residual 9.9464723774203639e-13
  • -x no -p right: residual 1.0545376349847963e-12
  • -x no -p left: residual 3.7412173369307980e-14
• Existing solver behavior remains unchanged: default flexible GMRES rejects left preconditioning and reports Flexible GMRES does not support left preconditioning. Use right preconditioning or disable flexible GMRES.

[shakedregev]

Please see how we use enums within ReSolve for SparseFormat and PermutationType and follow that. I've changed the target of your branch, as our CI tests don't work on mirrors. So once you make the changes, I will merge into this new target branch, and then if CLI passes, merge to develop.

[verma-divyanshu-git]

@shakedregev Updated to follow the existing enum usage pattern.

-p is now converted to Preconditioner::Side at the CLI boundary in both sysGmres and testSysGmres. Omitting -p still defaults to RIGHT; an explicit invalid value now reports the accepted values and exits with code 1. I also added an automated invalid-side functionality test and corrected the inherited warning-path logger call so the target builds.

CPU validation completed:

• pre-commit hooks passed
• warning-enabled build passed with -Wall -Wpedantic -Wconversion -Wextra
• make test: 51/51 passed
• make test_install: 1/1 passed
• default, right, and left functionality runs passed
• manual CPU examples produced residuals near machine precision

[verma-divyanshu-git]

@shakedregev Just wanted to follow-up: the requested enum-pattern changes are in d169084, and the branch is up to date.

Could you please take another look when you have a chance? The local CPU validation results are summarized in the comment above.

[verma-divyanshu-git]

hi @shakedregev, could you please take another look when you have a chance? the requested enum changes are in, the branch is mergeable, and the cpu checks passed; if it looks good, could you please clear the requested changes and merge it? thank you.

[pelesh]

@verma-divyanshu-git thank you for your contribution. If we are to make this change, it would need to be a wholesale change for SystemSolver I/O (see comments below).

[pelesh]

I don't think that we want to expose internal enum in SystemSolver API. The SystemSolver class is a wrapper for Re::Solve solvers and it is intended to simplify user interaction with the rest of the library. It takes string inputs as these are read from the command line interface.

I agree that it would be better to avoid string arguments in the SystemSolver class but that would then require expanding parser to select correct enumeration entries based on the CLI inputs. Should we decide to go that route, we should make appropriate changes to all string inputs, not only preconditioner type, to make the code consistent.