Skip to content

Enhancement: remove unnecessary second analyze #265

Merged
pelesh merged 4 commits intorefactor-examples-devfrom
shaked/do_one_analyze
May 8, 2025
Merged

Enhancement: remove unnecessary second analyze #265
pelesh merged 4 commits intorefactor-examples-devfrom
shaked/do_one_analyze

Conversation

@shakedregev
Copy link
Collaborator

@shakedregev shakedregev commented May 8, 2025

Description

kluRefactor, sysRefactor, and gpuRefactor were all performing a redundant second analyze.

Proposed changes

Made sure analyze is only done once. All tests pass.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here
to help! This is simply a reminder of what we are going to look for before
merging your code.

  • All tests pass. 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.

@shakedregev shakedregev requested a review from pelesh May 8, 2025 14:30
@pelesh
Copy link
Collaborator

pelesh commented May 8, 2025

So with this modification, the solution to system 01 fails for rocsolverRf and for cusolverRf. As expected, cusolverGLU works fine here. This is probably a bug in SystemSolver class when it decides to run triangular solve on GPU.

Copy link
Collaborator

@pelesh pelesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix to SystemSolver class was trivial so I pushed it here directly. Everything works fine now.

@pelesh pelesh merged commit de5aa51 into refactor-examples-dev May 8, 2025
4 checks passed
pelesh added a commit that referenced this pull request May 9, 2025
* Analysis is done only once in examples.

* Fix bug in SystemSolver class.

---------

Co-authored-by: Slaven Peles <peless@ornl.gov>
maksud pushed a commit that referenced this pull request May 9, 2025
* Analysis is done only once in examples.

* Fix bug in SystemSolver class.

---------

Co-authored-by: Slaven Peles <peless@ornl.gov>
pelesh added a commit that referenced this pull request May 12, 2025
* Better CLI input.

* Remove obsolete code.

* Read vectors directly from the file.

* Add print system info function to the helper.

* Rename r_KLU_rocSolverRf_FGMRES.cpp -> gpuRefactor.cpp

* Fix bug in system solver CUDA example.

* Rewrote GPU refactor so it works w/CUDA or HIP

Co-authored-by: Shaked Regev <shakedvregev@gmail.comregev@gmail.com>
Co-authored-by: pelesh <peless@ornl.gov>

* Refactor randomized GMRES examples

CPU, CUDA, HIP all work with the same source file.

Eliminated redundant code.

Added ExampleHelper to declutter the code and added details about how to use the various solvers.

* Enable turning off IR in gpuRefactor.

* Refactor examples for SystemSolver class (#252)


---------

Co-authored-by: Shaked Regev <shakedvregev@gmail.comregev@gmail.com>
Co-authored-by: Slaven Peles <peless@ornl.gov>

* Refactor and stand alone examples for KLU class (#253)

* Enhancement: remove unnecessary second analyze  (#265)

* Fix bug in SystemSolver class.

---------

Co-authored-by: Slaven Peles <peless@ornl.gov>

* Move experiemental examples to different directory.


---------

Co-authored-by: shakedregev <35384901+shakedregev@users.noreply.github.com>
Co-authored-by: Shaked Regev <shakedvregev@gmail.comregev@gmail.com>
pelesh added a commit that referenced this pull request May 13, 2025
* Better CLI input.

* Remove obsolete code.

* Read vectors directly from the file.

* Add print system info function to the helper.

* Rename r_KLU_rocSolverRf_FGMRES.cpp -> gpuRefactor.cpp

* Fix bug in system solver CUDA example.

* Rewrote GPU refactor so it works w/CUDA or HIP

Co-authored-by: Shaked Regev <shakedvregev@gmail.comregev@gmail.com>
Co-authored-by: pelesh <peless@ornl.gov>

* Refactor randomized GMRES examples

CPU, CUDA, HIP all work with the same source file.

Eliminated redundant code.

Added ExampleHelper to declutter the code and added details about how to use the various solvers.

* Enable turning off IR in gpuRefactor.

* Refactor examples for SystemSolver class (#252)


---------

Co-authored-by: Shaked Regev <shakedvregev@gmail.comregev@gmail.com>
Co-authored-by: Slaven Peles <peless@ornl.gov>

* Refactor and stand alone examples for KLU class (#253)

* Enhancement: remove unnecessary second analyze  (#265)

* Fix bug in SystemSolver class.

---------

Co-authored-by: Slaven Peles <peless@ornl.gov>

* Move experiemental examples to different directory.


---------

Co-authored-by: shakedregev <35384901+shakedregev@users.noreply.github.com>
Co-authored-by: Shaked Regev <shakedvregev@gmail.comregev@gmail.com>
@pelesh pelesh deleted the shaked/do_one_analyze branch July 24, 2025 20:49
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.

2 participants