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

Hard to read a reported value #109

Open
Tiehong opened this issue Mar 16, 2023 · 9 comments · May be fixed by #191
Open

Hard to read a reported value #109

Tiehong opened this issue Mar 16, 2023 · 9 comments · May be fixed by #191
Assignees

Comments

@Tiehong
Copy link

Tiehong commented Mar 16, 2023

In the report file, it will be better if a space is added between columns. It is hard to read a value if it is concatenated with another one (see below). Model is also attached.


Node Inflow Summary



                              Maximum  Maximum                  Lateral       Total        Flow
                              Lateral    Total  Time of Max      Inflow      Inflow     Balance
                               Inflow   Inflow   Occurrence      Volume      Volume       Error

Node Type CFS CFS days hr:min 10^6 gal 10^6 gal Percent

J1 JUNCTION 321741.00321741.00 0 11:54 5.72e+03 5.72e+03 0.000
J10 JUNCTION 33302.29 33310.07 0 11:54 589 594 0.001
J11 JUNCTION 0.00 200.01 0 05:21 0 109 0.004
J12 JUNCTION 47138.83 47138.83 0 11:54 838 838 0.000
J13 JUNCTION 0.00 7.65 0 03:12 0 3.66 0.006
J14 JUNCTION 0.00 112.79 0 04:30 0 62.8 0.041
J15 JUNCTION 0.00 0.00 0 00:00 0 0 0.000 gal
J16 JUNCTION 0.00 333.99 0 11:33 0 169 0.002
J17 JUNCTION 0.00 13.12 0 03:12 0 2.38 0.040
J18 JUNCTION 0.00 1.12 0 03:13 0 0.00463 0.000
J19 JUNCTION 217559.28217620.62 0 11:54 3.78e+03 3.81e+03 0.000
J2 JUNCTION 805208.19805208.19 0 11:54 1.43e+04 1.43e+04 0.000
J20 JUNCTION 1059209.661059209.66 0 11:54 1.85e+04 1.85e+04 0.000
J21 JUNCTION 11396023.9111396023.91 0 11:54 2.02e+05 2.02e+05 0.000
J22 JUNCTION 742677.23742678.50 0 11:54 1.32e+04 1.32e+04 0.000

working_LymanDrainage.zip

@cbuahin
Copy link
Collaborator

cbuahin commented Aug 9, 2023

@Tiehong , I am not able to reproduce your observation. The report file has columns that are space delimited. I am going to close this issue unless you are able to provide additional information.

@cbuahin cbuahin self-assigned this Aug 9, 2023
@MitchHeineman
Copy link

The problem arises when there are unusually large numbers in the result and the program does not convert them to scientific notation. Tiehong's example model does yield the behavior he described, as shown in the attached graphic.

image

@cbuahin
Copy link
Collaborator

cbuahin commented Aug 9, 2023

@MitchHeineman Interesting...

It seems to me this is an artifact of the GUI framework. Does resizing the column eliminate the issue or are the preceding numbers actually appended to the Type field in the example you show?

I will try to reproduce it on my end.

@MitchHeineman
Copy link

image

@cbuahin cbuahin added bug and removed bug labels Aug 9, 2023
@dickinsonre
Copy link

Wouldn't making all the columns scientific format fix the issue? Then there will be a space.

J1 JUNCTION 321741.00321741.00 0 11:54 5.72e+03 5.72e+03 0.000

@cbuahin
Copy link
Collaborator

cbuahin commented Aug 9, 2023

It seems to me this is an issue that has been addressed with various columns at various times. We probably have to think a bit about how we implement it going forward so that it works for all fields.

@cbuahin
Copy link
Collaborator

cbuahin commented Aug 9, 2023

We loose some of the digits using scientific format but it should be a good solution for now. It seems the larger issue is the attempt to align the column names with the values for ease in reading the results, which is a good thing for users.

cbuahin added a commit that referenced this issue Aug 9, 2023
@cbuahin cbuahin added this to the v5.2.5 milestone Aug 9, 2023
@MitchHeineman
Copy link

MitchHeineman commented Aug 10, 2023

I haven't looked at the code (besides, my native language is Fortran), and perhaps it already does this in places, but I'd suggest using floating point notation where values fit into the columns and scientific only for large values. In Fortran, this is generally a matter of using the G edit descriptor. Something like this?

#include <stdio.h>
#include <math.h>
void print_num(double num, int width)
{
int n = (int)log10(fabs(num)) + 1;
char buffer[width + 1];
if (n > width) {
if (width < 7) {
printf("%.s\n", width, "*****");;
return;
}
snprintf(buffer, width + 1, "%
.e", width -2 , width - 7, num);
} else {
snprintf(buffer, width + 1, "%
.*f", width , width - n - 1, num);
}
printf("%-*s\n", -width, buffer);
}

Now I see there's a G edit descriptor in C as well, but you get the idea...

cbuahin added a commit that referenced this issue Aug 28, 2023
@cbuahin cbuahin closed this as completed Oct 13, 2023
@michaeltryby michaeltryby reopened this Oct 8, 2024
@michaeltryby
Copy link
Collaborator

michaeltryby commented Oct 8, 2024

Reopening ...

Given that this issue is being reported at this late date, width overflow in report file tables can be regarded as an edge case. While the solution currently implemented, adopting the "%9.3g" print format over the "%9.3f" format has its merits, it changes the contents of report file tables in nearly all test cases.

On the otherhand, simply providing padding like so, " %8.3f" does not change report file table contents, will not result in truncation of flow values, and in the event of a width overflow inserts a space between fields. The only downside being that table formatting will not be maintained in the event of width overflow -- the current status quo. A more comprehensive solution would require an extensive rewrite of the code for printing report tables.

Therefore, to patch this bug, I prefer padding the fields with a space so they no longer run together in the event of a width overflow. No changes to table headers, the UI code, or the benchmark files is required.

PR coming forthwith ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants