Skip to content

Add estimations (inverse use of balance model) - #489

Open
lou-qui wants to merge 9 commits into
mainfrom
feature/#120/generic-estimations
Open

Add estimations (inverse use of balance model)#489
lou-qui wants to merge 9 commits into
mainfrom
feature/#120/generic-estimations

Conversation

@lou-qui

@lou-qui lou-qui commented Jul 2, 2026

Copy link
Copy Markdown
Member

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?

cf. #111

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)

adriengoeller and others added 2 commits June 22, 2026 17:19
Signed-off-by: GOELLER Adrien <adrien.goeller@rte-france.com>
@lou-qui lou-qui self-assigned this Jul 2, 2026
Copilot AI review requested due to automatic review settings July 2, 2026 13:47

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

Adds a new “estimation” (inverse balance-model) framework, exposing a generic EstimationEngine on top of SectionStudy plus root-finding methods to estimate environmental/load parameters that match a target obstacle distance.

Changes:

  • Introduces EstimationEngine with helpers to estimate temperature, wind pressure, and load mass by solving an inverse distance problem.
  • Adds optimization methods (Bisection/Brent/Newton) and a standardized EstimationResult.
  • Adds integration-style tests and exposes the engine via a lazy SectionStudy.estimation_engine property.

Reviewed changes

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

Show a summary per file
File Description
test/core/models/test_estimation.py Adds tests for EstimationResult, root-finding methods, and EstimationEngine behavior/state preservation.
src/mechaphlowers/core/models/estimation/result.py Defines the EstimationResult dataclass and __repr__.
src/mechaphlowers/core/models/estimation/methods.py Implements bisection, Brent (scipy-backed), and Newton solvers.
src/mechaphlowers/core/models/estimation/engine.py Implements inverse estimation logic over SectionStudy with state save/restore.
src/mechaphlowers/core/models/estimation/init.py Exposes the estimation public API.
src/mechaphlowers/api/section_study.py Adds a lazy-loaded estimation_engine property to SectionStudy.

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

Comment thread src/mechaphlowers/core/models/estimation/engine.py Outdated
Comment thread src/mechaphlowers/core/models/estimation/methods.py
Comment thread src/mechaphlowers/core/models/estimation/methods.py
Comment thread test/core/models/test_estimation.py
Comment thread test/core/models/test_estimation.py Outdated
lou-qui added 4 commits July 2, 2026 16:54
Signed-off-by: lou-qui <184963772+lou-qui@users.noreply.github.com>
Signed-off-by: lou-qui <184963772+lou-qui@users.noreply.github.com>
Signed-off-by: lou-qui <184963772+lou-qui@users.noreply.github.com>
Signed-off-by: lou-qui <184963772+lou-qui@users.noreply.github.com>
self.maxiter = maxiter
self.dx = dx

def solve(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why not use numeric.scipy.newton? (Or replace numeric.scipy.newton with this implementation?)

# file, You can obtain one at http://mozilla.org/MPL/2.0/.
# SPDX-License-Identifier: MPL-2.0

from __future__ import annotations

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why write this line? What is its effect?

# file, You can obtain one at http://mozilla.org/MPL/2.0/.
# SPDX-License-Identifier: MPL-2.0

from __future__ import annotations

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why write this line? What is its effect?

lou-qui added 2 commits July 6, 2026 15:29
Signed-off-by: lou-qui <184963772+lou-qui@users.noreply.github.com>
Co-Authored-By: Github Copilot
Signed-off-by: lou-qui <184963772+lou-qui@users.noreply.github.com>
def method(self, value: OptimizationMethod) -> None:
self._method = value

def estimate(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe rename into zero?

Comment on lines +60 to +66
@property
def method(self) -> OptimizationMethod:
return self._method

@method.setter
def method(self, value: OptimizationMethod) -> None:
self._method = value

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why define a getter and a setter?

wind_pressure: float | None = None,
ice_thickness: float | None = None,
) -> EstimationResult:
"""Find the cable temperature that yields a target distance to an obstacle.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is it possible to check the study has been "solved" before computing temperature?

obstacle_point: np.ndarray,
target_distance: float,
bounds: tuple[float, float] = (0.0, 200.0),
wind_pressure: float | None = None,

@lou-qui lou-qui Jul 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess this is only about the wind component that is perpendicular to the span frame?
I guess a wind_direction argument would be useful, same as .solve_change_state.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
79.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

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.

4 participants