Skip to content

Tolerate floating-point rounding in friction pressure-drop assert#3

Open
MatthiasBSchaefer wants to merge 4 commits into
masterfrom
MatthiasBSchaefer-PR4791
Open

Tolerate floating-point rounding in friction pressure-drop assert#3
MatthiasBSchaefer wants to merge 4 commits into
masterfrom
MatthiasBSchaefer-PR4791

Conversation

@MatthiasBSchaefer

@MatthiasBSchaefer MatthiasBSchaefer commented Jul 13, 2026

Copy link
Copy Markdown

Backport of modelica#4791 into this fork

Summary by CodeRabbit

Summary of changes

  • Bug Fixes

    • Improved validation of nominal pressure-drop parameters using a tolerance-adjusted check to prevent false failures from minor floating-point rounding differences.
  • Documentation / Chores

    • Added a small informational/reminder line to the header comment of a bundled C source component (no functional impact).

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e0a4f151-b9c1-45f5-93a1-0b61efcf48b9

📥 Commits

Reviewing files that changed from the base of the PR and between 8bad774 and de19205.

📒 Files selected for processing (1)
  • Modelica/Resources/C-Sources/ModelicaMatIO.c
🚧 Files skipped from review as they are similar to previous changes (1)
  • Modelica/Resources/C-Sources/ModelicaMatIO.c

📝 Walkthrough

Walkthrough

SimpleFriction now validates dpNominal against an epsilon-adjusted threshold, reformats pressure-drop equations, and adds an informational line to the ModelicaMatIO.c header comment.

Changes

Friction parameter validation

Layer / File(s) Summary
Relax nominal pressure-drop assertion
Modelica/Thermal/FluidHeatFlow/BaseClasses/SimpleFriction.mo
The initialization assertion uses an epsilon-adjusted lower bound, and pressure-drop expressions are reformatted without calculation changes.

Source header comment

Layer / File(s) Summary
Add source header remark
Modelica/Resources/C-Sources/ModelicaMatIO.c
An informational reminder line is added to the header comment without functional code changes.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: relaxing the friction pressure-drop assertion to tolerate floating-point rounding.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch MatthiasBSchaefer-PR4791

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.

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Removed an unnecessary remark from the comments.

@MSL-RegressionTesting-CI MSL-RegressionTesting-CI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@MSL-RegressionTesting-CI

Copy link
Copy Markdown

C code changes detected. Please add 'secure-c-code' label to the pr and rerun the workflow

@MatthiasBSchaefer MatthiasBSchaefer added the secure-c-code mark, that the changes in the c code are not malicious label Jul 23, 2026
@MSL-RegressionTesting-CI

Copy link
Copy Markdown

C code changes detected. Please add 'secure-c-code' label to the pr and rerun the workflow

1 similar comment
@MSL-RegressionTesting-CI

Copy link
Copy Markdown

C code changes detected. Please add 'secure-c-code' label to the pr and rerun the workflow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

secure-c-code mark, that the changes in the c code are not malicious

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants