Skip to content

Cable adjustment angles - #494

Open
wangjer wants to merge 4 commits into
mainfrom
feature/#140/cable-adjustment-angles
Open

Cable adjustment angles#494
wangjer wants to merge 4 commits into
mainfrom
feature/#140/cable-adjustment-angles

Conversation

@wangjer

@wangjer wangjer commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem?
Fixes #140

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The breaking change or deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

Other information:

wangjer added 2 commits July 22, 2026 17:09
Signed-off-by: WANG Jérémy <jeremy.wang@rte-france.com>
Signed-off-by: WANG Jérémy <jeremy.wang@rte-france.com>
@wangjer
wangjer marked this pull request as ready for review July 23, 2026 12:32
Copilot AI review requested due to automatic review settings July 23, 2026 12:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new “adjustment angles” computation model for cable adjustment use cases (Fixes #140) and adds initial unit tests validating expected angle outputs, while also correcting PAPOTO docstring parameter descriptions to match the angle-based API.

Changes:

  • Added compute_adjustment_angles algorithm implementation under core/models/adjustment_angles.
  • Added unit tests covering a couple of scalar scenarios and basic ndarray shape handling.
  • Updated papoto_3_points docstring to describe angles (not distances) for its parameters.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.

File Description
test/core/models/adjustment_angles/test_adjustment_model.py Adds unit tests validating expected adjustment angles and basic array support.
src/mechaphlowers/core/papoto/papoto_model.py Fixes docstring parameter descriptions to correctly refer to angles.
src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Adds the adjustment-angle computation implementation (with some robustness issues to address).
src/mechaphlowers/core/models/adjustment_angles/init.py Adds the package initializer for the new model module.
Comments suppressed due to low confidence (3)

src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py:82

  • The arccos input can drift slightly outside [-1, 1] due to floating-point roundoff, and dist_opposite_support can be ~0 for invalid geometries. Both cases will yield NaNs or warnings. Clipping and a small validation guard makes this more robust.
    expected_alpha_opposite_value = np.arccos(
        -(dist_support**2 - dist_opposite_support**2 - a**2)
        / 2
        / dist_opposite_support
        / a

src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py:104

  • The expression under the square root for d can become slightly negative due to rounding, producing NaNs. Using sqrt(max(x, 0)) avoids propagating floating error into invalid results.
    d = (
        (a / 2) ** 2
        + dist_left**2
        - 2 * dist_left * (a / 2) * np.cos(alpha_left)
    ) ** 0.5

src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py:109

  • As with the other arccos, the input here can drift slightly out of [-1, 1] from floating-point error, producing NaNs. Compute the cosine argument explicitly and clip it before calling arccos.
    expected_h_value = HR - np.arccos(
        -((a / 2) ** 2 - dist_right**2 - d**2) / d / dist_right / 2
    )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Outdated
Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Outdated
Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Outdated
Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py
Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Outdated
Comment thread test/core/models/adjustment_angles/test_adjustment_model.py
Signed-off-by: WANG Jérémy <jeremy.wang@rte-france.com>
@sonarqubecloud

Copy link
Copy Markdown

@lou-qui
lou-qui self-requested a review July 27, 2026 07:54

@lou-qui lou-qui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here are my first thoughts, I will complete later.

Thanks for letting me learn about the @overload syntax by the way.

Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Outdated
Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Outdated
Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py
Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Outdated
Comment thread src/mechaphlowers/core/models/adjustment_angles/adjustment_model.py Outdated
Comment thread test/core/models/adjustment_angles/test_adjustment_model.py
Signed-off-by: WANG Jérémy <jeremy.wang@rte-france.com>
@sonarqubecloud

Copy link
Copy Markdown

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.

Cables adjustments

3 participants