Skip to content

Implement get_parameters_at_bounds and suppress_warnings#170

Merged
henrikjacobsenfys merged 4 commits into
developfrom
get-parameters-at-bounds
May 4, 2026
Merged

Implement get_parameters_at_bounds and suppress_warnings#170
henrikjacobsenfys merged 4 commits into
developfrom
get-parameters-at-bounds

Conversation

@henrikjacobsenfys
Copy link
Copy Markdown
Member

@henrikjacobsenfys henrikjacobsenfys commented Apr 29, 2026

Closes #168
Closes #169

Co-authored-by: Copilot <copilot@github.com>
@henrikjacobsenfys henrikjacobsenfys added [scope] enhancement Adds/improves features (major.MINOR.patch) [priority] medium Normal/default priority labels Apr 29, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@7dbad54). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop     #170   +/-   ##
==========================================
  Coverage           ?   98.05%           
==========================================
  Files              ?       45           
  Lines              ?     2825           
  Branches           ?      496           
==========================================
  Hits               ?     2770           
  Misses             ?       33           
  Partials           ?       22           
Flag Coverage Δ
integration 48.92% <17.14%> (?)
unittests 98.05% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

henrikjacobsenfys and others added 2 commits April 29, 2026 13:56
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

Consider acting on silent ignore of NaNs.
The check could also be potentially re-made.

Comment on lines +461 to +465
if not isinstance(rtol, float):
raise TypeError(f'rtol must be a float. Got {type(rtol)}.')

if not isinstance(atol, float):
raise TypeError(f'atol must be a float. Got {type(atol)}.')
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.

Aren't those checks too strict?
get_parameters_near_bounds(rtol=0, atol=0) raises TypeError, even though np.isclose happily accepts ints. The same applies to numpy.float64 which is a float subclass and works, but plain Python int is rejected.

Maybe isinstance(x, (int, float)) and not isinstance(x, bool), or use numbers.Real. Also consider validating rtol >= 0 and atol >= 0 — negative tolerances will silently produce surprising results.

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.

or

from numbers import Real

if not isinstance(rtol, Real) or isinstance(rtol, bool) or rtol < 0:
    raise TypeError(f'rtol must be a non-negative number. Got {rtol!r}.')
if not isinstance(atol, Real) or isinstance(atol, bool) or atol < 0:
    raise TypeError(f'atol must be a non-negative number. Got {atol!r}.')

Comment on lines +471 to +472
value = p.value

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.

NaN values are silently ignored. If p.value is NaN, np.isclose will returns False, so a broken parameter isn't reported.

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.

would use a test for it

Co-authored-by: Copilot <copilot@github.com>
@henrikjacobsenfys henrikjacobsenfys changed the title Implement get_parameters_at_bounds Implement get_parameters_at_bounds and suppress_warnings Apr 29, 2026
Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

looks good now!

@henrikjacobsenfys henrikjacobsenfys merged commit 6cc5b8d into develop May 4, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[priority] medium Normal/default priority [scope] enhancement Adds/improves features (major.MINOR.patch)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a "suppress_warnings" setting Add a get_parameters_at_bounds type function that indicate if parameters have hit their bounds

2 participants