Skip to content

Conversation

@networmix
Copy link
Owner

Summary

  • avoid floating point drift when placing demand

Testing

  • pytest --cov=ngraph --cov-fail-under=85 --cov-report term-missing

Copy link

Copilot AI left a comment

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 enhances demand placement by introducing deterministic rounding to mitigate floating-point drift.

  • Added a _round_float helper to snap tiny values to zero.
  • Applied rounding to placed_demand, placed_now, and remaining in place().
  • Imported math and MIN_FLOW to support the new logic.
Comments suppressed due to low confidence (2)

ngraph/lib/demand.py:110

  • remaining is computed using the unrounded placed_now, which can reintroduce floating-point drift. Consider rounding placed_now first and then calculating remaining based on the rounded value.
remaining = to_place - placed_now

ngraph/lib/demand.py:27

  • New rounding behavior in _round_float should have dedicated unit tests covering finite values, non-finite values, and the MIN_FLOW threshold to ensure correctness.
def _round_float(value: float) -> float:

Comment on lines +26 to +30
@staticmethod
def _round_float(value: float) -> float:
"""Round ``value`` to avoid tiny floating point drift."""
if math.isfinite(value):
rounded = round(value, 12)
Copy link

Copilot AI May 18, 2025

Choose a reason for hiding this comment

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

[nitpick] The precision value 12 is a magic number. Extract this into a named constant (e.g., ROUND_PRECISION) or make it configurable to improve clarity and maintainability.

Suggested change
@staticmethod
def _round_float(value: float) -> float:
"""Round ``value`` to avoid tiny floating point drift."""
if math.isfinite(value):
rounded = round(value, 12)
# Precision for rounding floating-point values
ROUND_PRECISION: int = 12
@staticmethod
def _round_float(value: float) -> float:
"""Round ``value`` to avoid tiny floating point drift."""
if math.isfinite(value):
rounded = round(value, Demand.ROUND_PRECISION)

Copilot uses AI. Check for mistakes.
@networmix networmix merged commit 87c4455 into main May 18, 2025
2 checks passed
@networmix networmix deleted the codex/run-pytest-and-fix-failing-test branch May 18, 2025 05:14
networmix added a commit that referenced this pull request Jun 2, 2025
* adding transforms

* Fix floating point rounding in Demand.place (#65)

* Add CLI tool with run command (#67)
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