Skip to content

Conversation

@Bot-Enigma-0
Copy link
Contributor

Proposed Changes

Give a brief overview of your contribution here in a few sentences.
We have a non-linear system described by x_{n+1} = F(x_n), where x_n and x_{n+1} are the solution states at time steps n and n+1, and F represents our nonlinear solver iteration. The spectral radius of the Jacobian matrix J_F = dF/dx determines the stability of our iterative scheme.

The power iteration method allows us to estimate the dominant eigenvalue (spectral radius) without explicitly forming the Jacobian matrix. The basic idea is:

  1. Start with a random vector v
  2. Repeatedly compute w = J_F * v (using forward-mode AD)
  3. Normalize w to get the new v
  4. The norm of w converges to the dominant eigenvalue of the system

The current version has some bugs leading to seg faults which i'm trying to debug. I would appreciate any comments.

Related Work

Resolve any issues (bug fix or feature request), note any related PRs, or mention interactions with the work of others, if any.

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.

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

Appart from the nullptr issue with nodes. and missing mpi reduction, this looks correct 👍

* \brief Seed derivatives for all solution variables using a flat vector.
* \param[in] derivatives - Flat vector of derivative values (size nVar * nPoint)
*/
void SetSolutionDerivatives(const vector<passivedouble>& derivatives, CVariable* nodes) {
Copy link
Member

Choose a reason for hiding this comment

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

use su2matrix instead of vector passivedouble so you don't need to deal with the offset variable

Comment on lines 336 to 339
norm += v_estimate[i] * v_estimate[i];
}
norm = sqrt(norm);
for (unsigned long i = 0; i < nTotal; i++) v_estimate[i] /= norm;
Copy link
Member

Choose a reason for hiding this comment

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

For multiple mpi ranks you need to reduce the norm and then take the square root and normalize the vector.

}

// Seed derivatives for all solvers
SeedAllDerivatives(v_estimate, nodes, geometry);
Copy link
Member

Choose a reason for hiding this comment

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

nodes is nullptr in these calls. You don't need to pass nodes here, the solver owns its nodes.

unsigned long nPoint = geom_ptr->GetnPointDomain();

//requested solver and variable
unsigned short targetSolver = 2;

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable targetSolver is not used.

Copilot Autofix

AI about 1 month ago

The best way to fix the problem is to remove the declaration of the unused local variable targetSolver from the CFluidIteration::PreRunSpectralRadius function in SU2_CFD/src/iteration/CFluidIteration.cpp. No changes to functionality or imports are necessary as the variable is completely unused. The code can be safely deleted from line 781 with no other modifications required.

Suggested changeset 1
SU2_CFD/src/iteration/CFluidIteration.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/SU2_CFD/src/iteration/CFluidIteration.cpp b/SU2_CFD/src/iteration/CFluidIteration.cpp
--- a/SU2_CFD/src/iteration/CFluidIteration.cpp
+++ b/SU2_CFD/src/iteration/CFluidIteration.cpp
@@ -778,7 +778,7 @@
   unsigned long nPoint = geom_ptr->GetnPointDomain();
   
   //requested solver and variable
-  unsigned short targetSolver = 2;
+
   unsigned short targetVar = 0;
   unsigned long nVarTotal = 1;
 
EOF
@@ -778,7 +778,7 @@
unsigned long nPoint = geom_ptr->GetnPointDomain();

//requested solver and variable
unsigned short targetSolver = 2;

unsigned short targetVar = 0;
unsigned long nVarTotal = 1;

Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated

//requested solver and variable
unsigned short targetSolver = 2;
unsigned short targetVar = 0;

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable targetVar is not used.

Copilot Autofix

AI about 1 month ago

To fix the problem, remove the declaration unsigned short targetVar = 0; on line 782 in SU2_CFD/src/iteration/CFluidIteration.cpp. Since it's completely unused, removal improves readability and prevents any confusion. No other changes to code logic are needed, as no parts of the function refer to this variable.

Suggested changeset 1
SU2_CFD/src/iteration/CFluidIteration.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/SU2_CFD/src/iteration/CFluidIteration.cpp b/SU2_CFD/src/iteration/CFluidIteration.cpp
--- a/SU2_CFD/src/iteration/CFluidIteration.cpp
+++ b/SU2_CFD/src/iteration/CFluidIteration.cpp
@@ -779,7 +779,6 @@
   
   //requested solver and variable
   unsigned short targetSolver = 2;
-  unsigned short targetVar = 0;
   unsigned long nVarTotal = 1;
 
     //initialize power iteration matrix
EOF
@@ -779,7 +779,6 @@

//requested solver and variable
unsigned short targetSolver = 2;
unsigned short targetVar = 0;
unsigned long nVarTotal = 1;

//initialize power iteration matrix
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
@pcarruscag pcarruscag mentioned this pull request Oct 4, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants