Skip to content

feat(cfd-python): expose blake threshold and giersiepen models via PyO3#265

Open
ryancinsight wants to merge 1 commit intomainfrom
fix/python-bindings-for-TODOs-14562815834805710155
Open

feat(cfd-python): expose blake threshold and giersiepen models via PyO3#265
ryancinsight wants to merge 1 commit intomainfrom
fix/python-bindings-for-TODOs-14562815834805710155

Conversation

@ryancinsight
Copy link
Owner

@ryancinsight ryancinsight commented Mar 12, 2026

Expose physics functions to cfd-python to close out TODO gaps in the validation script and cross-validate Rust/Python implementations.


PR created automatically by Jules for task 14562815834805710155 started by @ryancinsight

High-level PR Summary

This PR exposes Blake threshold cavitation calculations and Giersiepen hemolysis models from the Rust cfd-core library to Python via PyO3 bindings. The changes add two new Python wrapper classes (CavitationRegimeClassifier and GiersiepenModel) and updates the validation script to cross-validate the Rust and Python implementations, replacing TODO comments with actual test cases that verify numerical equivalence between the two implementations.

⏱️ Estimated Review Time: 5-15 minutes

💡 Review Order Suggestion
Order File Path
1 .gitignore
2 crates/cfd-core/src/physics/cavitation/regimes/classifier.rs
3 crates/cfd-python/src/hemolysis.rs
4 crates/cfd-python/src/cavitation.rs
5 crates/cfd-python/src/lib.rs
6 validation/cross_validate_rust_python.py

Need help? Join our Discord

Summary by CodeRabbit

  • New Features
    • Added Python API for cavitation regime classification, enabling Blake threshold and critical radius calculations.
    • Added Python API for hemolysis damage calculations using the Giersiepen model with shear stress and exposure time parameters.

- Add `CavitationRegimeClassifier` binding to `cfd-python` (exposing `blake_threshold` and `blake_critical_radius`).
- Add `GiersiepenModel` binding to `cfd-python` (exposing `calculate_damage`).
- Make `bubble_model`, `ambient_pressure`, `acoustic_pressure`, and `acoustic_frequency` public in `cfd-core`'s `CavitationRegimeClassifier`.
- Remove TODO tags from `validation/cross_validate_rust_python.py` and replace them with actual `cfd_python` cross-validation tests that compare Python analytical values against Rust implementation. All tests pass with < 0.01% error.
- Add `venv/`, `.venv/`, and `__pycache__/` to `.gitignore`.

Co-authored-by: ryancinsight <55164720+ryancinsight@users.noreply.github.com>
@google-labs-jules
Copy link
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request integrates key computational fluid dynamics (CFD) physics models, specifically the Blake threshold for cavitation and the Giersiepen model for hemolysis, into the cfd-python library. This integration allows Python users to directly access and utilize these Rust-implemented models, facilitating cross-language validation and expanding the functionality of the Python binding.

Highlights

  • Python Bindings for Physics Models: Exposed the Blake threshold cavitation model and the Giersiepen hemolysis model to Python via PyO3, allowing direct access to these Rust-implemented physics functions from Python.
  • API Visibility Update: Modified the CavitationRegimeClassifier struct in cfd-core to make its internal fields publicly accessible, facilitating its use in the Python bindings.
  • New Python Modules: Introduced dedicated Python modules (cavitation.rs and hemolysis.rs) within cfd-python to encapsulate the new physics function bindings.
  • Enhanced Cross-Validation Script: Updated the cross_validate_rust_python.py script to include automated cross-validation tests for the newly exposed Blake threshold and Giersiepen hemolysis models, ensuring consistency between Rust and Python implementations.
  • Git Ignore Update: Added Python virtual environment (.venv/, venv/) and cache (__pycache__/) directories to .gitignore to prevent them from being tracked.
Changelog
  • .gitignore
    • Added entries for Python virtual environment and cache directories.
  • crates/cfd-core/src/physics/cavitation/regimes/classifier.rs
    • Modified visibility of CavitationRegimeClassifier fields from pub(super) to pub.
  • crates/cfd-python/src/cavitation.rs
    • Added a new module exposing PyCavitationRegimeClassifier with methods for Blake threshold and critical radius calculations.
  • crates/cfd-python/src/hemolysis.rs
    • Added a new module exposing PyGiersiepenModel with a method to calculate hemolysis damage.
  • crates/cfd-python/src/lib.rs
    • Integrated the new cavitation and hemolysis modules and registered their PyO3 classes.
  • validation/cross_validate_rust_python.py
    • Updated the validation script to include cross-checks for the newly exposed Blake threshold and Giersiepen hemolysis models, removing previous TODO comments.
Activity
  • The pull request was automatically created by Jules for task [14562815834805710155], initiated by @ryancinsight.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist
Copy link
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

This pull request expands Python API surface through PyO3 bindings for cavitation regime classification and hemolysis modeling, increases visibility of cavitation classifier fields from pub(super) to pub, updates project ignore patterns, and extends validation with cross-validation logic comparing Rust and Python implementations.

Changes

Cohort / File(s) Summary
Configuration
.gitignore
Adds ignore patterns for Python virtual environments (.venv/, venv/) and cache directories (__pycache__/).
Physics Core - Visibility
crates/cfd-core/src/physics/cavitation/regimes/classifier.rs
Increases visibility of four struct fields (bubble_model, ambient_pressure, acoustic_pressure, acoustic_frequency) in CavitationRegimeClassifier<T> from pub(super) to pub.
Python Bindings - Cavitation
crates/cfd-python/src/cavitation.rs
Introduces PyCavitationRegimeClassifier wrapper exposing constructor with physical parameters, blake_threshold() and blake_critical_radius() methods, and string representations.
Python Bindings - Hemolysis
crates/cfd-python/src/hemolysis.rs
Introduces PyGiersiepenModel wrapper with constructor, calculate_damage(shear_stress, exposure_time) method with error handling, and string representations.
Python Bindings - Registration
crates/cfd-python/src/lib.rs
Adds module declarations for cavitation and hemolysis, exports new Python-facing classes PyCavitationRegimeClassifier and PyGiersiepenModel, and registers them during module initialization.
Validation & Testing
validation/cross_validate_rust_python.py
Extends cross-validation script with runtime API-driven checks for Blake threshold, Carreau-Yasuda blood viscosity, and Giersiepen hemolysis model, comparing Rust and Python implementations with percent-error thresholds.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hops through PyO3's delight,
Cavitation bubbles wrapped just right,
Hemolysis models bridge the tongue so fair,
From Rust to Python through the air,
Validation bonds them with a gleeful cheer!
🌾✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: exposing blake threshold calculations and the giersiepen hemolysis model to Python via PyO3 bindings, which aligns with the primary objectives of adding new PyO3 wrappers for physics functions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/python-bindings-for-TODOs-14562815834805710155

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
validation/cross_validate_rust_python.py (1)

236-239: ⚠️ Potential issue | 🟡 Minor

Remove outdated "NEXT STEP" guidance.

The summary still includes guidance about adding cross-validation tests, but these tests were just implemented in this PR. This text is now outdated and misleading.

🧹 Proposed fix to update summary
 3. ✓ Giersiepen Hemolysis
    - Python: validation/validate_cavitation_hemolysis.py line 160-168
    - Rust: crates/cfd-core/src/physics/hemolysis/giersiepen.rs
    - Formula: D = C×τ^α×t^β

-NEXT STEP: Add explicit cross-validation tests that:
-  - Call Rust via cfd_python with same inputs
-  - Compare outputs to Python calculations
-  - Assert < 0.01% difference
+Cross-validation tests above compare Rust vs Python with <0.01% tolerance.
 """)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@validation/cross_validate_rust_python.py` around lines 236 - 239, The file
contains an outdated "NEXT STEP: Add explicit cross-validation tests..."
guidance string in the summary; remove that obsolete line so the
module/class/function summary no longer suggests work that's already done
(search for the exact "NEXT STEP: Add explicit cross-validation tests" text and
delete it), leaving current documentation and any remaining summary text
accurate and up to date (ensure no other occurrences remain).
🧹 Nitpick comments (2)
.gitignore (1)

42-46: Remove the duplicated ignore patterns.

Lines 45-46 repeat entries already added on Lines 42-43, so they don't change ignore behavior and just add noise.

♻️ Proposed cleanup
 .venv/
 __pycache__/
 venv/
-.venv/
-__pycache__/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.gitignore around lines 42 - 46, The .gitignore contains duplicate patterns
(".venv/", "__pycache__/", "venv/") repeated twice; remove the second
occurrences so each ignore pattern appears only once (e.g., delete the redundant
lines that repeat ".venv/", "__pycache__/", "venv/") leaving a single set of
those entries to reduce noise and keep the file clean.
crates/cfd-python/src/hemolysis.rs (1)

36-42: Consider enriching __repr__ with model parameters.

The current representation is minimal. For debugging and interactive use, it could be helpful to include the model constants.

♻️ Optional enhancement for richer representation
     fn __str__(&self) -> String {
         "GiersiepenModel()".to_string()
     }

     fn __repr__(&self) -> String {
-        self.__str__()
+        "GiersiepenModel(C=3.62e-5, α=2.416, β=0.785)".to_string()
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/cfd-python/src/hemolysis.rs` around lines 36 - 42, The __repr__
currently returns the same minimal string as __str__ ("GiersiepenModel()");
update GiersiepenModel::__repr__ to include the model's parameter values (e.g.,
the struct fields/constants) so that repr shows a concise, unambiguous snapshot
like GiersiepenModel{param1: x, param2: y}. Locate the GiersiepenModel impl
where fn __str__ and fn __repr__ are defined and build a formatted string in
__repr__ that reads the model's fields (use the actual field names from the
struct) and returns that string instead of delegating to __str__.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@validation/cross_validate_rust_python.py`:
- Around line 187-209: Remove unnecessary f-string prefixes on print statements
that contain no placeholders: change prints like print(f"  Located in:
crates/..."), print(f"  Cross-validation:"), print(f"  ✓ MATCH for all test
cases (<0.01% diff)"), and print(f"  Skipping test: GiersiepenModel not found in
cfd_python") to plain string prints. Locate the block guarded by
hasattr(cfd_python, 'GiersiepenModel') around the GiersiepenModel usage and the
loop over test_cases (which calls giersiepen_python and model.calculate_damage)
and update only the string literals to remove the leading f for consistency and
clarity. Ensure any print that still needs interpolation (e.g., the mismatch
message using {tau}, {t}, etc.) remains an f-string.
- Around line 131-148: The print statements that use f-strings but contain no
placeholders should be normal strings: update the prints in the
CarreauYasudaBlood cross-validation block (the "  Cross-validation:", the
success message "  ✓ MATCH for all shear rates (<0.01% diff)" and the skip
message "  Skipping test: CarreauYasudaBlood not found in cfd_python") to remove
the leading f prefix while keeping the gamma character in the mismatch message
intact; locate the block guarded by hasattr(cfd_python, 'CarreauYasudaBlood')
and adjust the print calls so only the mismatch line that formats
mu_python/mu_rust remains an f-string and other static messages use plain string
literals.
- Around line 84-88: The two print statements that use f-strings but contain no
placeholders should be converted to plain strings: update the print in the
branch that prints "  Cross-validation: ✓ MATCH (<0.01% diff)" and the print in
the else that prints "  Skipping test: CavitationRegimeClassifier not found in
cfd_python" to remove the leading f prefix (leave the other print that uses
{rc_error} and {pb_error} unchanged); locate these prints near the
cross-validation logic in cross_validate_rust_python.py (the prints used
alongside CavitationRegimeClassifier handling) and replace f"..." with "..." so
static analysis Ruff F541 is satisfied.
- Around line 60-61: Remove the unrelated outer hasattr(cfd_python,
'CavitySolver2D') guard so the Blake threshold test only checks for
CavitationRegimeClassifier; modify the block around the
CavitationRegimeClassifier check (the code that currently sits under the
CavitySolver2D if) to directly use if hasattr(cfd_python,
'CavitationRegimeClassifier') and shift the corresponding else clause
indentation to match that check (ensure the expected test/skip behavior is
preserved for the CavitationRegimeClassifier branch).

---

Outside diff comments:
In `@validation/cross_validate_rust_python.py`:
- Around line 236-239: The file contains an outdated "NEXT STEP: Add explicit
cross-validation tests..." guidance string in the summary; remove that obsolete
line so the module/class/function summary no longer suggests work that's already
done (search for the exact "NEXT STEP: Add explicit cross-validation tests" text
and delete it), leaving current documentation and any remaining summary text
accurate and up to date (ensure no other occurrences remain).

---

Nitpick comments:
In @.gitignore:
- Around line 42-46: The .gitignore contains duplicate patterns (".venv/",
"__pycache__/", "venv/") repeated twice; remove the second occurrences so each
ignore pattern appears only once (e.g., delete the redundant lines that repeat
".venv/", "__pycache__/", "venv/") leaving a single set of those entries to
reduce noise and keep the file clean.

In `@crates/cfd-python/src/hemolysis.rs`:
- Around line 36-42: The __repr__ currently returns the same minimal string as
__str__ ("GiersiepenModel()"); update GiersiepenModel::__repr__ to include the
model's parameter values (e.g., the struct fields/constants) so that repr shows
a concise, unambiguous snapshot like GiersiepenModel{param1: x, param2: y}.
Locate the GiersiepenModel impl where fn __str__ and fn __repr__ are defined and
build a formatted string in __repr__ that reads the model's fields (use the
actual field names from the struct) and returns that string instead of
delegating to __str__.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 98965a34-0029-457e-be24-50e09daeed10

📥 Commits

Reviewing files that changed from the base of the PR and between d0aa6c2 and fdc5f23.

📒 Files selected for processing (6)
  • .gitignore
  • crates/cfd-core/src/physics/cavitation/regimes/classifier.rs
  • crates/cfd-python/src/cavitation.rs
  • crates/cfd-python/src/hemolysis.rs
  • crates/cfd-python/src/lib.rs
  • validation/cross_validate_rust_python.py

Comment on lines +60 to +61
if hasattr(cfd_python, 'CavitySolver2D'):
# Check if the required API is exposed
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Redundant check for CavitySolver2D before CavitationRegimeClassifier.

The outer hasattr(cfd_python, 'CavitySolver2D') check appears unrelated to the Blake threshold test. This may be a copy-paste artifact. The test should only check for CavitationRegimeClassifier.

🐛 Proposed fix to remove unrelated check
 if has_cfd_python:
     print(f"\nRust implementation:")
     print(f"  Located in: crates/cfd-core/src/physics/cavitation/regimes.rs")

-    if hasattr(cfd_python, 'CavitySolver2D'):
-        # Check if the required API is exposed
-        if hasattr(cfd_python, 'CavitationRegimeClassifier'):
+    # Check if the required API is exposed
+    if hasattr(cfd_python, 'CavitationRegimeClassifier'):

And adjust the corresponding else clause indentation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@validation/cross_validate_rust_python.py` around lines 60 - 61, Remove the
unrelated outer hasattr(cfd_python, 'CavitySolver2D') guard so the Blake
threshold test only checks for CavitationRegimeClassifier; modify the block
around the CavitationRegimeClassifier check (the code that currently sits under
the CavitySolver2D if) to directly use if hasattr(cfd_python,
'CavitationRegimeClassifier') and shift the corresponding else clause
indentation to match that check (ensure the expected test/skip behavior is
preserved for the CavitationRegimeClassifier branch).

Comment on lines +84 to +88
print(f" Cross-validation: ✓ MATCH (<0.01% diff)")
else:
print(f" Cross-validation: ✗ MISMATCH (Rc err: {rc_error:.4f}%, Pb err: {pb_error:.4f}%)")
else:
print(f" Skipping test: CavitationRegimeClassifier not found in cfd_python")
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unnecessary f prefix from strings without placeholders.

Static analysis (Ruff F541) correctly identifies f-strings that contain no placeholders. These should be plain strings.

🧹 Proposed fix for lines 84 and 88
             if rc_error < 0.01 and pb_error < 0.01:
-                print(f"  Cross-validation: ✓ MATCH (<0.01% diff)")
+                print("  Cross-validation: ✓ MATCH (<0.01% diff)")
             else:
                 print(f"  Cross-validation: ✗ MISMATCH (Rc err: {rc_error:.4f}%, Pb err: {pb_error:.4f}%)")
         else:
-            print(f"  Skipping test: CavitationRegimeClassifier not found in cfd_python")
+            print("  Skipping test: CavitationRegimeClassifier not found in cfd_python")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
print(f" Cross-validation: ✓ MATCH (<0.01% diff)")
else:
print(f" Cross-validation: ✗ MISMATCH (Rc err: {rc_error:.4f}%, Pb err: {pb_error:.4f}%)")
else:
print(f" Skipping test: CavitationRegimeClassifier not found in cfd_python")
print(" Cross-validation: ✓ MATCH (<0.01% diff)")
else:
print(f" Cross-validation: ✗ MISMATCH (Rc err: {rc_error:.4f}%, Pb err: {pb_error:.4f}%)")
else:
print(" Skipping test: CavitationRegimeClassifier not found in cfd_python")
🧰 Tools
🪛 Ruff (0.15.5)

[error] 84-84: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 88-88: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@validation/cross_validate_rust_python.py` around lines 84 - 88, The two print
statements that use f-strings but contain no placeholders should be converted to
plain strings: update the print in the branch that prints "  Cross-validation: ✓
MATCH (<0.01% diff)" and the print in the else that prints "  Skipping test:
CavitationRegimeClassifier not found in cfd_python" to remove the leading f
prefix (leave the other print that uses {rc_error} and {pb_error} unchanged);
locate these prints near the cross-validation logic in
cross_validate_rust_python.py (the prints used alongside
CavitationRegimeClassifier handling) and replace f"..." with "..." so static
analysis Ruff F541 is satisfied.

Comment on lines +131 to +148
if hasattr(cfd_python, 'CarreauYasudaBlood'):
blood = cfd_python.CarreauYasudaBlood()
print(f" Cross-validation:")

all_match = True
for gamma_dot in test_shear_rates:
mu_python = carreau_yasuda_python(gamma_dot)
mu_rust = blood.apparent_viscosity(gamma_dot)
error_pct = abs(mu_rust - mu_python) / mu_python * 100

if error_pct > 0.01:
all_match = False
print(f" Mismatch at γ̇ = {gamma_dot}: Python={mu_python*1000:.4f}, Rust={mu_rust*1000:.4f} (err: {error_pct:.2f}%)")

if all_match:
print(f" ✓ MATCH for all shear rates (<0.01% diff)")
else:
print(f" Skipping test: CarreauYasudaBlood not found in cfd_python")
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unnecessary f prefixes; gamma character is acceptable.

Lines 133, 146, and 148 have f-strings without placeholders. The ambiguous gamma character (γ̇) at line 143 is intentional scientific notation for shear rate and should be kept.

🧹 Proposed fix for f-string issues
     if hasattr(cfd_python, 'CarreauYasudaBlood'):
         blood = cfd_python.CarreauYasudaBlood()
-        print(f"  Cross-validation:")
+        print("  Cross-validation:")

         all_match = True
         for gamma_dot in test_shear_rates:
             mu_python = carreau_yasuda_python(gamma_dot)
             mu_rust = blood.apparent_viscosity(gamma_dot)
             error_pct = abs(mu_rust - mu_python) / mu_python * 100

             if error_pct > 0.01:
                 all_match = False
                 print(f"    Mismatch at γ̇ = {gamma_dot}: Python={mu_python*1000:.4f}, Rust={mu_rust*1000:.4f} (err: {error_pct:.2f}%)")

         if all_match:
-            print(f"  ✓ MATCH for all shear rates (<0.01% diff)")
+            print("  ✓ MATCH for all shear rates (<0.01% diff)")
     else:
-        print(f"  Skipping test: CarreauYasudaBlood not found in cfd_python")
+        print("  Skipping test: CarreauYasudaBlood not found in cfd_python")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if hasattr(cfd_python, 'CarreauYasudaBlood'):
blood = cfd_python.CarreauYasudaBlood()
print(f" Cross-validation:")
all_match = True
for gamma_dot in test_shear_rates:
mu_python = carreau_yasuda_python(gamma_dot)
mu_rust = blood.apparent_viscosity(gamma_dot)
error_pct = abs(mu_rust - mu_python) / mu_python * 100
if error_pct > 0.01:
all_match = False
print(f" Mismatch at γ̇ = {gamma_dot}: Python={mu_python*1000:.4f}, Rust={mu_rust*1000:.4f} (err: {error_pct:.2f}%)")
if all_match:
print(f" ✓ MATCH for all shear rates (<0.01% diff)")
else:
print(f" Skipping test: CarreauYasudaBlood not found in cfd_python")
if hasattr(cfd_python, 'CarreauYasudaBlood'):
blood = cfd_python.CarreauYasudaBlood()
print(" Cross-validation:")
all_match = True
for gamma_dot in test_shear_rates:
mu_python = carreau_yasuda_python(gamma_dot)
mu_rust = blood.apparent_viscosity(gamma_dot)
error_pct = abs(mu_rust - mu_python) / mu_python * 100
if error_pct > 0.01:
all_match = False
print(f" Mismatch at γ̇ = {gamma_dot}: Python={mu_python*1000:.4f}, Rust={mu_rust*1000:.4f} (err: {error_pct:.2f}%)")
if all_match:
print(" ✓ MATCH for all shear rates (<0.01% diff)")
else:
print(" Skipping test: CarreauYasudaBlood not found in cfd_python")
🧰 Tools
🪛 Ruff (0.15.5)

[error] 133-133: f-string without any placeholders

Remove extraneous f prefix

(F541)


[warning] 143-143: String contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?

(RUF001)


[error] 146-146: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 148-148: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@validation/cross_validate_rust_python.py` around lines 131 - 148, The print
statements that use f-strings but contain no placeholders should be normal
strings: update the prints in the CarreauYasudaBlood cross-validation block (the
"  Cross-validation:", the success message "  ✓ MATCH for all shear rates
(<0.01% diff)" and the skip message "  Skipping test: CarreauYasudaBlood not
found in cfd_python") to remove the leading f prefix while keeping the gamma
character in the mismatch message intact; locate the block guarded by
hasattr(cfd_python, 'CarreauYasudaBlood') and adjust the print calls so only the
mismatch line that formats mu_python/mu_rust remains an f-string and other
static messages use plain string literals.

Comment on lines +187 to +209
print(f" Located in: crates/cfd-core/src/physics/hemolysis/models.rs")

if hasattr(cfd_python, 'GiersiepenModel'):
model = cfd_python.GiersiepenModel()
print(f" Cross-validation:")

all_match = True
for tau, t in test_cases:
damage_python = giersiepen_python(tau, t)
damage_rust = model.calculate_damage(tau, t)

# Note: Rust model might return slightly different results based on float precision or slight variations in constants
# so we use a reasonable tolerance
error_pct = abs(damage_rust - damage_python) / damage_python * 100

if error_pct > 0.01:
all_match = False
print(f" Mismatch at τ={tau}, t={t}: Python={damage_python:.6f}, Rust={damage_rust:.6f} (err: {error_pct:.2f}%)")

if all_match:
print(f" ✓ MATCH for all test cases (<0.01% diff)")
else:
print(f" Skipping test: GiersiepenModel not found in cfd_python")
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unnecessary f prefixes from strings without placeholders.

🧹 Proposed fix for f-string issues in TEST 3
     print(f"\nRust implementation:")
-    print(f"  Located in: crates/cfd-core/src/physics/hemolysis/models.rs")
+    print("  Located in: crates/cfd-core/src/physics/hemolysis/models.rs")

     if hasattr(cfd_python, 'GiersiepenModel'):
         model = cfd_python.GiersiepenModel()
-        print(f"  Cross-validation:")
+        print("  Cross-validation:")

         all_match = True
         for tau, t in test_cases:
             damage_python = giersiepen_python(tau, t)
             damage_rust = model.calculate_damage(tau, t)

             # Note: Rust model might return slightly different results based on float precision or slight variations in constants
             # so we use a reasonable tolerance
             error_pct = abs(damage_rust - damage_python) / damage_python * 100

             if error_pct > 0.01:
                 all_match = False
                 print(f"    Mismatch at τ={tau}, t={t}: Python={damage_python:.6f}, Rust={damage_rust:.6f} (err: {error_pct:.2f}%)")

         if all_match:
-            print(f"  ✓ MATCH for all test cases (<0.01% diff)")
+            print("  ✓ MATCH for all test cases (<0.01% diff)")
     else:
-        print(f"  Skipping test: GiersiepenModel not found in cfd_python")
+        print("  Skipping test: GiersiepenModel not found in cfd_python")
🧰 Tools
🪛 Ruff (0.15.5)

[error] 187-187: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 191-191: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 207-207: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 209-209: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@validation/cross_validate_rust_python.py` around lines 187 - 209, Remove
unnecessary f-string prefixes on print statements that contain no placeholders:
change prints like print(f"  Located in: crates/..."), print(f" 
Cross-validation:"), print(f"  ✓ MATCH for all test cases (<0.01% diff)"), and
print(f"  Skipping test: GiersiepenModel not found in cfd_python") to plain
string prints. Locate the block guarded by hasattr(cfd_python,
'GiersiepenModel') around the GiersiepenModel usage and the loop over test_cases
(which calls giersiepen_python and model.calculate_damage) and update only the
string literals to remove the leading f for consistency and clarity. Ensure any
print that still needs interpolation (e.g., the mismatch message using {tau},
{t}, etc.) remains an f-string.

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.

1 participant