Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reduce tolerances in benchmark model gradient checks #2511

Open
dweindl opened this issue Sep 27, 2024 · 2 comments
Open

Reduce tolerances in benchmark model gradient checks #2511

dweindl opened this issue Sep 27, 2024 · 2 comments
Labels
ci

Comments

@dweindl
Copy link
Member

dweindl commented Sep 27, 2024 •
edited
Loading

The current tolerances are huge:

AMICI/tests/benchmark-models/test_petab_benchmark.py

Lines 14 to 16 in bea2dfe

# Absolute and relative tolerances for finite difference gradient checks.
ATOL: float = 1e-3
RTOL: float = 1e-2

@dweindl dweindl added the ci label Sep 27, 2024
@dweindl dweindl mentioned this issue Sep 27, 2024
Merged
@dweindl
Copy link
Member Author

dweindl commented Sep 30, 2024 •
edited
Loading

Worse... failed gradient checks would not have been reported at all. success is type fiddy.derivative_check.DerivativeCheckResult.

AMICI/tests/benchmark-models/test_petab_benchmark.py

Line 159 in bea2dfe

success = check(rtol=RTOL, atol=ATOL)

We need to check the success attribute, not the full object here:

AMICI/tests/benchmark-models/test_petab_benchmark.py

Line 184 in bea2dfe

assert success, derivative.df

dweindl added a commit that referenced this issue Oct 2, 2024
@dweindl
Benchmark model tests: fix gradients checks
8bd63f3 
* Fix gradient checks. Previously, failing test would not have been reported (see #2511).
  Quite some tests are now skipped, but better to test some than none... To be re-enabled later.
* Sort test cases to avoid issues with pytest-xdist.
* Refactor

Requires ICB-DCM/fiddy#37
@dweindl
Copy link
Member Author

dweindl commented Oct 8, 2024 •
edited
Loading

For Blasi_CellSystems2016 (nothing special in the model, except for post-equilibration), adjoints are so far off, that I think there is something wrong ...

pretty good: tests/benchmark-models/test_petab_benchmark.py::test_benchmark_gradient[Blasi_CellSystems2016-forward-scaled]
pretty bad: tests/benchmark-models/test_petab_benchmark.py::test_benchmark_gradient[Blasi_CellSystems2016-adjoint-scaled]

EDIT: The problem is SteadyStateSensitivityMode.integrateIfNewtonFails. With ASA, this fails badly here. SteadyStateSensitivityMode.integrationOnly works.

@dweindl dweindl mentioned this issue Oct 9, 2024
Merged
dweindl added a commit that referenced this issue Oct 9, 2024
@dweindl
CI: Re-enable benchmark model gradient checks / prettify gradient che…
6261491 
…ck output (#2524)

* Make the result more informative and more readable.
* Re-enable some test problems, use stricter tolerances for some at least (#2511) 
* More problem-specific settings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ci
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dweindl