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

Fix _print_value_fmt for some objectives and fix some latex labels #1230

Merged
merged 3 commits into from
Aug 28, 2024

Conversation

YigitElma
Copy link
Collaborator

@YigitElma YigitElma commented Aug 28, 2024

#1189 changed the format of _print_value_fmt some recent merged PR's added new objectives that shouldn't work with the new format and throw IndexError: Replacement index 2 out of range for positional args tuple.

Also, I think I missed one of the objectives during that PR. This fixes the formats.

And some small label fixes.

@YigitElma YigitElma added easy Short and simple to code or review bug fix Something was fixed labels Aug 28, 2024
@YigitElma YigitElma requested review from a team, rahulgaur104, f0uriest, ddudt, dpanici, kianorr, sinaatalay and unalmis and removed request for a team August 28, 2024 18:20
@YigitElma YigitElma changed the title Fix _print_value_fmt for some objectives Fix _print_value_fmt for some objectives Aug 28, 2024
@unalmis
Copy link
Collaborator

unalmis commented Aug 28, 2024

Can you also fix <|B|>_axis and B_phi|r,t latex labels in this pr?

unalmis
unalmis previously approved these changes Aug 28, 2024
@rahulgaur104
Copy link
Collaborator

Are these all the objectives in the current master where the formatting specification is used after _print_value_fmt?

@YigitElma
Copy link
Collaborator Author

Are these all the objectives in the current master where the formatting specification is used after _print_value_fmt?

I searched _print_value_fmt = on VS Code and checked the strings. These should be all of them, unless a PR gets merged without checking the print.

@YigitElma
Copy link
Collaborator Author

Can you also fix <|B|>_axis and B_phi|r,t latex labels in this pr?

What is this exactly? You can add your commit here if it is a quick fix

rahulgaur104
rahulgaur104 previously approved these changes Aug 28, 2024
@unalmis unalmis dismissed stale reviews from rahulgaur104 and themself via b348886 August 28, 2024 18:50
@YigitElma YigitElma changed the title Fix _print_value_fmt for some objectives Fix _print_value_fmt for some objectives and fix some latex labels Aug 28, 2024
Copy link

codecov bot commented Aug 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.33%. Comparing base (04e6e56) to head (63ed69e).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1230      +/-   ##
==========================================
- Coverage   95.33%   95.33%   -0.01%     
==========================================
  Files          90       90              
  Lines       22702    22702              
==========================================
- Hits        21644    21642       -2     
- Misses       1058     1060       +2     
Files with missing lines Coverage Δ
desc/compute/_equil.py 100.00% <ø> (ø)
desc/compute/_field.py 100.00% <ø> (ø)
desc/objectives/_equilibrium.py 94.95% <100.00%> (ø)
desc/objectives/_power_balance.py 89.58% <100.00%> (ø)

... and 1 file with indirect coverage changes

Copy link
Contributor

github-actions bot commented Aug 28, 2024

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     -2.26 +/- 7.79     | -1.18e-02 +/- 4.07e-02 |  5.10e-01 +/- 3.5e-02  |  5.22e-01 +/- 2.0e-02  |
 test_build_transform_fft_midres         |     -1.40 +/- 3.01     | -8.47e-03 +/- 1.82e-02 |  5.96e-01 +/- 5.9e-03  |  6.04e-01 +/- 1.7e-02  |
 test_build_transform_fft_highres        |     -1.15 +/- 1.81     | -1.14e-02 +/- 1.80e-02 |  9.82e-01 +/- 6.8e-03  |  9.94e-01 +/- 1.7e-02  |
 test_equilibrium_init_lowres            |     -1.59 +/- 1.27     | -6.05e-02 +/- 4.83e-02 |  3.74e+00 +/- 2.0e-02  |  3.80e+00 +/- 4.4e-02  |
 test_equilibrium_init_medres            |     -4.02 +/- 3.46     | -1.70e-01 +/- 1.47e-01 |  4.06e+00 +/- 3.3e-02  |  4.23e+00 +/- 1.4e-01  |
 test_equilibrium_init_highres           |     -1.15 +/- 1.44     | -6.25e-02 +/- 7.83e-02 |  5.39e+00 +/- 2.9e-02  |  5.45e+00 +/- 7.3e-02  |
 test_objective_compile_dshape_current   |     +0.63 +/- 0.85     | +2.41e-02 +/- 3.22e-02 |  3.83e+00 +/- 3.1e-02  |  3.81e+00 +/- 7.8e-03  |
 test_objective_compile_atf              |     -0.44 +/- 2.53     | -3.44e-02 +/- 1.99e-01 |  7.84e+00 +/- 1.4e-01  |  7.87e+00 +/- 1.4e-01  |
 test_objective_compute_dshape_current   |     -0.72 +/- 2.09     | -2.53e-05 +/- 7.30e-05 |  3.47e-03 +/- 4.6e-05  |  3.49e-03 +/- 5.6e-05  |
 test_objective_compute_atf              |     +4.42 +/- 4.07     | +4.54e-04 +/- 4.18e-04 |  1.07e-02 +/- 4.0e-04  |  1.03e-02 +/- 1.2e-04  |
 test_objective_jac_dshape_current       |     +1.52 +/- 6.02     | +6.29e-04 +/- 2.49e-03 |  4.20e-02 +/- 1.6e-03  |  4.13e-02 +/- 1.9e-03  |
 test_objective_jac_atf                  |     +4.25 +/- 3.24     | +8.45e-02 +/- 6.46e-02 |  2.08e+00 +/- 4.6e-02  |  1.99e+00 +/- 4.5e-02  |
 test_perturb_1                          |     +2.78 +/- 3.71     | +3.39e-01 +/- 4.51e-01 |  1.25e+01 +/- 4.5e-01  |  1.22e+01 +/- 6.3e-02  |
 test_perturb_2                          |     +6.42 +/- 2.55     | +1.09e+00 +/- 4.35e-01 |  1.81e+01 +/- 3.6e-01  |  1.70e+01 +/- 2.5e-01  |
 test_proximal_jac_atf                   |     +1.97 +/- 4.32     | +1.63e-01 +/- 3.57e-01 |  8.44e+00 +/- 3.5e-01  |  8.28e+00 +/- 6.4e-02  |
 test_proximal_freeb_compute             |     -0.67 +/- 0.97     | -1.23e-03 +/- 1.78e-03 |  1.82e-01 +/- 1.6e-03  |  1.83e-01 +/- 6.9e-04  |
 test_proximal_freeb_jac                 |     +1.23 +/- 1.65     | +9.14e-02 +/- 1.23e-01 |  7.55e+00 +/- 1.2e-01  |  7.46e+00 +/- 4.0e-02  |
 test_solve_fixed_iter                   |     +2.21 +/- 63.81    | +1.12e-01 +/- 3.23e+00 |  5.18e+00 +/- 2.5e+00  |  5.07e+00 +/- 2.1e+00  |

@YigitElma YigitElma merged commit f21d65b into master Aug 28, 2024
18 checks passed
@unalmis unalmis deleted the yge/hotfix branch August 28, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Something was fixed easy Short and simple to code or review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants