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.

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

Improve warning for missing IOable attributes #1095

Merged
merged 6 commits into from
Jul 4, 2024
Merged

Conversation

unalmis
Copy link
Collaborator

@unalmis unalmis commented Jul 3, 2024

No description provided.

@unalmis unalmis added the documentation Add documentation or better warnings etc. label Jul 3, 2024
rahulgaur104
rahulgaur104 previously approved these changes Jul 3, 2024
desc/io/hdf5_io.py Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Jul 3, 2024

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     +7.50 +/- 15.20    | +3.77e-02 +/- 7.64e-02 |  5.41e-01 +/- 7.6e-02  |  5.03e-01 +/- 1.1e-02  |
 test_build_transform_fft_midres         |     +1.54 +/- 3.62     | +9.07e-03 +/- 2.13e-02 |  5.98e-01 +/- 2.1e-02  |  5.89e-01 +/- 3.2e-03  |
 test_build_transform_fft_highres        |     +5.22 +/- 3.40     | +5.10e-02 +/- 3.32e-02 |  1.03e+00 +/- 3.2e-02  |  9.76e-01 +/- 7.7e-03  |
 test_equilibrium_init_lowres            |     +1.03 +/- 2.10     | +3.74e-02 +/- 7.59e-02 |  3.66e+00 +/- 7.5e-02  |  3.62e+00 +/- 1.3e-02  |
 test_equilibrium_init_medres            |     +0.01 +/- 0.47     | +2.58e-04 +/- 1.92e-02 |  4.09e+00 +/- 1.5e-02  |  4.09e+00 +/- 1.2e-02  |
 test_equilibrium_init_highres           |     +0.22 +/- 0.96     | +1.18e-02 +/- 5.24e-02 |  5.49e+00 +/- 3.5e-02  |  5.47e+00 +/- 3.9e-02  |
 test_objective_compile_dshape_current   |     +0.10 +/- 1.96     | +3.79e-03 +/- 7.33e-02 |  3.74e+00 +/- 7.0e-02  |  3.74e+00 +/- 2.1e-02  |
 test_objective_compile_atf              |     +0.75 +/- 2.25     | +6.03e-02 +/- 1.81e-01 |  8.14e+00 +/- 6.1e-02  |  8.08e+00 +/- 1.7e-01  |
 test_objective_compute_dshape_current   |     -0.94 +/- 3.54     | -1.18e-05 +/- 4.43e-05 |  1.24e-03 +/- 3.5e-05  |  1.25e-03 +/- 2.7e-05  |
 test_objective_compute_atf              |     -0.54 +/- 6.38     | -2.24e-05 +/- 2.66e-04 |  4.15e-03 +/- 2.1e-04  |  4.18e-03 +/- 1.7e-04  |
 test_objective_jac_dshape_current       |     +1.83 +/- 8.64     | +6.94e-04 +/- 3.27e-03 |  3.86e-02 +/- 2.8e-03  |  3.79e-02 +/- 1.8e-03  |
 test_objective_jac_atf                  |     +2.35 +/- 2.55     | +4.34e-02 +/- 4.72e-02 |  1.89e+00 +/- 3.3e-02  |  1.85e+00 +/- 3.3e-02  |
 test_perturb_1                          |     -0.06 +/- 0.83     | -8.37e-03 +/- 1.07e-01 |  1.29e+01 +/- 9.1e-02  |  1.29e+01 +/- 5.7e-02  |
 test_perturb_2                          |     +0.05 +/- 1.21     | +8.66e-03 +/- 2.16e-01 |  1.79e+01 +/- 1.4e-01  |  1.79e+01 +/- 1.6e-01  |
 test_proximal_jac_atf                   |     -0.46 +/- 0.86     | -3.39e-02 +/- 6.31e-02 |  7.29e+00 +/- 5.4e-02  |  7.33e+00 +/- 3.2e-02  |
 test_proximal_freeb_compute             |     -0.21 +/- 0.88     | -3.74e-04 +/- 1.56e-03 |  1.77e-01 +/- 6.5e-04  |  1.77e-01 +/- 1.4e-03  |
 test_proximal_freeb_jac                 |     +0.13 +/- 1.19     | +9.53e-03 +/- 8.74e-02 |  7.33e+00 +/- 7.0e-02  |  7.32e+00 +/- 5.2e-02  |
 test_solve_fixed_iter                   |     +0.09 +/- 9.94     | +1.35e-02 +/- 1.45e+00 |  1.46e+01 +/- 1.1e+00  |  1.46e+01 +/- 9.8e-01  |

@YigitElma
Copy link
Collaborator

@unalmis Since it is a bit late for the USA, I fixed the problem that caused the tests to fail. I believe everything should pass now.

@rahulgaur104 rahulgaur104 self-requested a review July 3, 2024 06:12
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.98%. Comparing base (fd2645a) to head (880e074).
Report is 2120 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1095   +/-   ##
=======================================
  Coverage   94.98%   94.98%           
=======================================
  Files          87       87           
  Lines       21748    21749    +1     
=======================================
+ Hits        20658    20659    +1     
  Misses       1090     1090           
Files with missing lines Coverage Δ
desc/io/hdf5_io.py 81.96% <100.00%> (+0.09%) ⬆️

YigitElma
YigitElma previously approved these changes Jul 3, 2024
rahulgaur104
rahulgaur104 previously approved these changes Jul 3, 2024
f0uriest
f0uriest previously approved these changes Jul 3, 2024
YigitElma
YigitElma previously approved these changes Jul 3, 2024
Copy link
Collaborator

@dpanici dpanici left a comment

Choose a reason for hiding this comment

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

It is not an "input file", it is really an "output file" since it is an .h5, maybe instead say "file being loaded".

@unalmis unalmis dismissed stale reviews from YigitElma and f0uriest via 288c17b July 3, 2024 20:05
@unalmis unalmis requested review from ddudt and dpanici July 3, 2024 20:05
@unalmis unalmis merged commit 0a6b995 into master Jul 4, 2024
18 checks passed
@unalmis unalmis deleted the better_warning branch July 4, 2024 00:06
@unalmis unalmis self-assigned this Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Add documentation or better warnings etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants