Skip to content

Conversation

@shbhmexe
Copy link

Proposed Changes

This PR addresses code quality issues across the SU2 codebase:

Typo Fixes (Comment-only)

  1. "approxiations" → "approximations" in 5 C++ solver files
  2. "generilize" → "generalize" in CEulerSolver.cpp (4 occurrences)

Python Code Style (PEP8)

  1. Replace type(x) == list with isinstance(x, list) in gradients.py and functions.py

C++ Safety Improvements

  1. Add zero-distance guard in CHeatSolver.cpp (lines 640, 671)

    • Changed /dist to /fmax(dist, EPS) to prevent potential division by zero
  2. Remove redundant 1.0* multiplication in CRadP1Solver.cpp (lines 323, 397)

C++ Bug Fixes (Common)

  1. Fix float literal in CSysMatrix.cpp (line 160)

    • Changed (nPointDomain + 1.0) to (nPointDomain + 1) for GPU array allocation
    • Using float for array sizing is incorrect type usage
  2. Remove redundant 1.0* in wall_model.cpp (line 374)

Related Work

No related issues. This is a cleanup PR to improve code quality and robustness.

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings.
  • My contribution is commented and consistent with SU2 style.
  • I used the pre-commit hook to prevent dirty commits.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation, if necessary.

Impact

  • Typo fixes: No functional changes (comment-only)
  • Python style: Functionally equivalent, follows PEP8
  • CHeatSolver.cpp: Adds safety for edge cases
  • CSysMatrix.cpp: Fixes type correctness for GPU memory allocation
  • CRadP1Solver.cpp & wall_model.cpp: Code cleanup

- Fix typo 'approxiations' -> 'approximations' in solver comments
- Fix typo 'generilize' -> 'generalize' in CEulerSolver.cpp
- Replace type(x) == list with isinstance(x, list) per PEP8
- Add zero-distance guard in CHeatSolver.cpp Heat_Fluxes function
- Remove redundant 1.0* multiplication in CRadP1Solver.cpp
- Fix float literal 1.0 to integer 1 in CSysMatrix.cpp GPU allocation
- Remove redundant 1.0* multiplication in wall_model.cpp

Signed-off-by: shbhmexe <[email protected]>
@shbhmexe shbhmexe changed the base branch from master to develop December 26, 2025 08:05
dist = sqrt(dist);

dTdn = (Twall - nodes->GetTemperature(iPointNormal))/dist;
dTdn = (Twall - nodes->GetTemperature(iPointNormal))/fmax(dist, EPS);
Copy link
Member

Choose a reason for hiding this comment

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

Not necessary here and in line 671, if the coordinate is the same there are bigger problems

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review! You're right - I've reverted the fmax(dist, EPS) changes on both lines 640 and 671. If the coordinate is the same, that would indeed indicate a bigger problem that should be addressed elsewhere rather than masked by a safety guard here.


# check for multiple objectives
multi_objective = type(func_name) == list
multi_objective = isinstance(func_name, list)
Copy link
Member

Choose a reason for hiding this comment

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

Since you are looking into the python scripts, do you think you can help with #2587 ?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the ping @pcarruscag . I've taken a look at PR #2587. Since the original author seems inactive, how would you prefer I proceed?

I can verify these changes locally and open a fresh PR with the updates rebased on the latest develop branch if that works best for you. Let me know!

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