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

Add easier to read print option #1189

Merged
merged 55 commits into from
Aug 24, 2024
Merged

Add easier to read print option #1189

merged 55 commits into from
Aug 24, 2024

Conversation

YigitElma
Copy link
Collaborator

@YigitElma YigitElma commented Aug 13, 2024

Add an option to print start and end errors next to each other for easier comparison.
Resolves #1112

==============================================================================================================
                                                       Start  -->   End
Total (sum of squares)                             6.877e-12  -->   6.877e-12, 
Maximum absolute Force error:                      7.852e+00  -->   7.852e+00 (N)
Minimum absolute Force error:                      1.009e-05  -->   1.009e-05 (N)
Average absolute Force error:                      1.148e-01  -->   1.148e-01 (N)
Maximum absolute Force error:                      2.546e-06  -->   2.546e-06 (normalized)
Minimum absolute Force error:                      3.274e-12  -->   3.274e-12 (normalized)
Average absolute Force error:                      3.722e-08  -->   3.722e-08 (normalized)
R boundary error:                                  0.000e+00  -->   0.000e+00 (m)
Z boundary error:                                  0.000e+00  -->   0.000e+00 (m)
Fixed Psi error:                                   0.000e+00  -->   0.000e+00 (Wb)
Fixed pressure profile error:                      0.000e+00  -->   0.000e+00 (Pa)
Fixed iota profile error:                          0.000e+00  -->   0.000e+00 (dimensionless)
Fixed sheet current error:                         0.000e+00  -->   0.000e+00 (~)
==============================================================================================================
  • Add a test that checks length of print_value_fmt of each objective and ensures that they are shorter than print_result_width value

@YigitElma YigitElma requested review from dpanici and f0uriest August 13, 2024 22:33
Copy link
Contributor

github-actions bot commented Aug 13, 2024

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     -0.94 +/- 11.25    | -4.93e-03 +/- 5.91e-02 |  5.20e-01 +/- 3.7e-02  |  5.25e-01 +/- 4.6e-02  |
 test_build_transform_fft_midres         |     -2.28 +/- 7.86     | -1.41e-02 +/- 4.86e-02 |  6.04e-01 +/- 2.2e-02  |  6.18e-01 +/- 4.3e-02  |
 test_build_transform_fft_highres        |     -2.44 +/- 6.55     | -2.54e-02 +/- 6.81e-02 |  1.01e+00 +/- 2.8e-02  |  1.04e+00 +/- 6.2e-02  |
 test_equilibrium_init_lowres            |     +2.10 +/- 7.64     | +8.11e-02 +/- 2.95e-01 |  3.94e+00 +/- 2.3e-01  |  3.86e+00 +/- 1.9e-01  |
 test_equilibrium_init_medres            |     -5.90 +/- 5.72     | -2.71e-01 +/- 2.63e-01 |  4.32e+00 +/- 1.3e-01  |  4.59e+00 +/- 2.3e-01  |
 test_equilibrium_init_highres           |     -4.92 +/- 3.25     | -2.96e-01 +/- 1.95e-01 |  5.71e+00 +/- 1.5e-01  |  6.01e+00 +/- 1.2e-01  |
 test_objective_compile_dshape_current   |     -1.41 +/- 1.66     | -5.70e-02 +/- 6.71e-02 |  3.98e+00 +/- 5.6e-02  |  4.04e+00 +/- 3.7e-02  |
-test_objective_compile_atf              |     +8.41 +/- 2.55     | +6.57e-01 +/- 1.99e-01 |  8.47e+00 +/- 1.9e-01  |  7.81e+00 +/- 5.7e-02  |
 test_objective_compute_dshape_current   |    +11.41 +/- 6.10     | +3.95e-04 +/- 2.11e-04 |  3.86e-03 +/- 2.0e-04  |  3.46e-03 +/- 6.7e-05  |
-test_objective_compute_atf              |    +14.87 +/- 2.44     | +1.52e-03 +/- 2.50e-04 |  1.17e-02 +/- 2.2e-04  |  1.02e-02 +/- 1.3e-04  |
 test_objective_jac_dshape_current       |     +8.63 +/- 7.15     | +3.48e-03 +/- 2.88e-03 |  4.38e-02 +/- 2.7e-03  |  4.03e-02 +/- 1.0e-03  |
 test_objective_jac_atf                  |     +4.34 +/- 3.23     | +8.32e-02 +/- 6.18e-02 |  2.00e+00 +/- 3.5e-02  |  1.91e+00 +/- 5.1e-02  |
-test_perturb_1                          |    +13.67 +/- 1.85     | +1.66e+00 +/- 2.25e-01 |  1.38e+01 +/- 2.1e-01  |  1.22e+01 +/- 7.8e-02  |
 test_perturb_2                          |     +5.27 +/- 4.82     | +9.13e-01 +/- 8.36e-01 |  1.82e+01 +/- 8.2e-01  |  1.73e+01 +/- 1.8e-01  |
 test_proximal_jac_atf                   |     +1.14 +/- 1.46     | +9.24e-02 +/- 1.19e-01 |  8.23e+00 +/- 1.1e-01  |  8.13e+00 +/- 4.8e-02  |
 test_proximal_freeb_compute             |     +2.02 +/- 1.94     | +3.68e-03 +/- 3.53e-03 |  1.85e-01 +/- 2.9e-03  |  1.82e-01 +/- 2.0e-03  |
 test_proximal_freeb_jac                 |     +1.22 +/- 2.17     | +9.02e-02 +/- 1.60e-01 |  7.47e+00 +/- 1.3e-01  |  7.38e+00 +/- 9.6e-02  |
 test_solve_fixed_iter                   |     +0.21 +/- 58.95    | +1.04e-02 +/- 2.91e+00 |  4.94e+00 +/- 2.0e+00  |  4.93e+00 +/- 2.1e+00  |

desc/optimize/optimizer.py Outdated Show resolved Hide resolved
@dpanici dpanici requested review from a team, rahulgaur104, f0uriest, ddudt, dpanici, kianorr and sinaatalay and removed request for a team, dpanici and f0uriest August 14, 2024 00:08
@YigitElma YigitElma marked this pull request as draft August 14, 2024 00:45
@unalmis
Copy link
Collaborator

unalmis commented Aug 19, 2024

if you want a nitpick, my personal preference is one line of dashes "-" to a line of "= and/or +". either way thanks for doing this

@YigitElma
Copy link
Collaborator Author

if you want a nitpick, my personal preference is one line of dashes "-" to a line of "= and/or +". either way thanks for doing this

I tried - first, it looked a bit weak. At least for me, = acts better up to its name as divider 😄

@YigitElma
Copy link
Collaborator Author

This looks so much better!

I think the line breaks \n are excessive since we already separate the sections with the diving line ====, but I'm fine with it if everyone else prefers having the line breaks

It is again up to the general vote, but I feel like, especially when you run something like solve_continuation_automatic, our stdouts get too long and I would like to make them modularized.

f0uriest
f0uriest previously approved these changes Aug 19, 2024
@unalmis unalmis dismissed their stale review August 19, 2024 21:14

requested changes were made

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.

I am going to be annoying but could we have a line break btwn objectives so you can tell them apart? otherwise it is just like 100 numbers and they are far from the objective names that say what each are...

dpanici
dpanici previously approved these changes Aug 20, 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.

Nevermind, the units help distinguish

@dpanici
Copy link
Collaborator

dpanici commented Aug 21, 2024

run black and flake8 on repo then we will commit

@YigitElma YigitElma dismissed stale reviews from dpanici and f0uriest via bccf5fd August 21, 2024 19:55
@YigitElma YigitElma added the easy Short and simple to code or review label Aug 22, 2024
@YigitElma YigitElma merged commit 4281a96 into master Aug 24, 2024
18 checks passed
@YigitElma YigitElma deleted the yge/print branch August 24, 2024 01:46
YigitElma added a commit that referenced this pull request Aug 28, 2024
…1230)

#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.
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.

Have Objective Before/After costs be displayed Concurrently for Each Objective
5 participants