Skip to content

Conversation

@dpanici
Copy link
Collaborator

@dpanici dpanici commented Aug 11, 2025

Resolves #1831

@dpanici dpanici requested review from a team, YigitElma, ddudt, f0uriest, rahulgaur104 and unalmis and removed request for a team August 11, 2025 21:25
@dpanici dpanici added the easy Short and simple to code or review label Aug 11, 2025
@dpanici dpanici added the run_benchmarks Run timing benchmarks on this PR against current master branch label Aug 11, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2025

Memory benchmark result

|               Test Name                |      %Δ      |    Master (MB)     |      PR (MB)       |    Δ (MB)    |    Time PR (s)     |  Time Master (s)   |
| -------------------------------------- | ------------ | ------------------ | ------------------ | ------------ | ------------------ | ------------------ |
  test_objective_jac_w7x                 |    6.25 %    |     3.843e+03      |     4.084e+03      |    240.34    |       33.94        |       31.23        |
- test_proximal_jac_w7x_with_eq_update   |   19.83 %    |     5.713e+03      |     6.846e+03      |   1133.09    |       160.30       |       53.58        |
  test_proximal_freeb_jac                |   -0.23 %    |     1.320e+04      |     1.317e+04      |    -30.03    |       77.52        |       77.72        |
  test_proximal_freeb_jac_blocked        |   -0.66 %    |     7.666e+03      |     7.615e+03      |    -50.71    |       68.59        |       68.60        |
  test_proximal_freeb_jac_batched        |    0.62 %    |     7.595e+03      |     7.642e+03      |    47.35     |       68.07        |       68.99        |
  test_proximal_jac_ripple               |    2.79 %    |     7.475e+03      |     7.683e+03      |    208.23    |       69.41        |       71.33        |
  test_proximal_jac_ripple_spline        |    0.09 %    |     3.471e+03      |     3.474e+03      |     3.23     |       71.30        |       71.22        |
  test_eq_solve                          |   -3.31 %    |     2.073e+03      |     2.004e+03      |    -68.59    |       125.16       |       125.07       |

For the memory plots, go to the summary of Memory Benchmarks workflow and download the artifact.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2025

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_midres         |     +0.22 +/- 2.46     | +1.47e-03 +/- 1.63e-02 |  6.65e-01 +/- 8.3e-03  |  6.63e-01 +/- 1.4e-02  |
 test_build_transform_fft_highres        |     -0.15 +/- 3.87     | -1.36e-03 +/- 3.50e-02 |  9.02e-01 +/- 2.6e-02  |  9.03e-01 +/- 2.4e-02  |
 test_equilibrium_init_lowres            |     -2.88 +/- 1.79     | -1.29e-01 +/- 8.00e-02 |  4.34e+00 +/- 4.9e-02  |  4.46e+00 +/- 6.3e-02  |
 test_objective_compile_atf              |     -3.50 +/- 3.62     | -2.18e-01 +/- 2.25e-01 |  6.00e+00 +/- 1.6e-01  |  6.22e+00 +/- 1.6e-01  |
 test_objective_compute_atf              |     -7.20 +/- 17.63    | -1.59e-04 +/- 3.89e-04 |  2.05e-03 +/- 2.5e-04  |  2.21e-03 +/- 3.0e-04  |
 test_objective_jac_atf                  |     -1.62 +/- 2.26     | -2.80e-02 +/- 3.90e-02 |  1.70e+00 +/- 2.4e-02  |  1.73e+00 +/- 3.0e-02  |
 test_perturb_1                          |     -4.57 +/- 3.00     | -6.48e-01 +/- 4.26e-01 |  1.35e+01 +/- 3.7e-01  |  1.42e+01 +/- 2.1e-01  |
 test_proximal_jac_atf                   |     -0.52 +/- 1.47     | -2.89e-02 +/- 8.23e-02 |  5.55e+00 +/- 6.6e-02  |  5.58e+00 +/- 4.9e-02  |
 test_proximal_freeb_compute             |     -1.24 +/- 2.95     | -2.02e-03 +/- 4.79e-03 |  1.60e-01 +/- 3.2e-03  |  1.62e-01 +/- 3.6e-03  |
 test_solve_fixed_iter                   |     -3.89 +/- 2.33     | -1.15e+00 +/- 6.89e-01 |  2.84e+01 +/- 2.9e-01  |  2.95e+01 +/- 6.2e-01  |
 test_objective_compute_ripple           |     -0.34 +/- 1.04     | -8.93e-03 +/- 2.72e-02 |  2.61e+00 +/- 2.0e-02  |  2.61e+00 +/- 1.9e-02  |
 test_objective_grad_ripple              |     -0.37 +/- 2.48     | -1.76e-02 +/- 1.17e-01 |  4.68e+00 +/- 4.6e-02  |  4.69e+00 +/- 1.1e-01  |

@unalmis unalmis marked this pull request as draft August 13, 2025 01:06
@codecov
Copy link

codecov bot commented Aug 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.81%. Comparing base (7e334c3) to head (7fa9267).
⚠️ Report is 61 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1842      +/-   ##
==========================================
- Coverage   95.81%   95.81%   -0.01%     
==========================================
  Files         100      100              
  Lines       27536    27539       +3     
==========================================
+ Hits        26384    26386       +2     
- Misses       1152     1153       +1     
Files with missing lines Coverage Δ
desc/optimize/_constraint_wrappers.py 97.31% <100.00%> (+0.02%) ⬆️

... and 3 files with indirect coverage changes

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

@dpanici dpanici marked this pull request as ready for review August 13, 2025 19:15
@dpanici dpanici requested review from a team and YigitElma and removed request for a team August 25, 2025 20:19
Copy link
Collaborator

@YigitElma YigitElma left a comment

Choose a reason for hiding this comment

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

Hmm, I don't see how we can prevent the failing benchmarks since the kwarg is recently added. If you can merge it by overriding somehow, I am fine with it.

Should update the changelog though

@dpanici dpanici removed the run_benchmarks Run timing benchmarks on this PR against current master branch label Aug 26, 2025
@YigitElma YigitElma added run_benchmarks Run timing benchmarks on this PR against current master branch and removed run_benchmarks Run timing benchmarks on this PR against current master branch labels Sep 4, 2025
@dpanici dpanici added run_benchmarks Run timing benchmarks on this PR against current master branch and removed run_benchmarks Run timing benchmarks on this PR against current master branch labels Sep 5, 2025
@YigitElma YigitElma merged commit 01c8bc8 into master Sep 7, 2025
27 checks passed
@YigitElma YigitElma deleted the dp/prox-always-solve-first branch September 7, 2025 16:25
@unalmis unalmis mentioned this pull request Sep 10, 2025
unalmis added a commit that referenced this pull request Sep 10, 2025
Reruns the optimization to see changes due to #1842.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

easy Short and simple to code or review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ensure or recommend that equilibrium is solved (eq.solve) before beginning an optimization (eq.optimize)

4 participants