Redefine Trim* and Round* functions in terms of {fmt} #7772
Conversation
Problem:
* Most of the diffs that come up during replacement of GIO with {fmt}
involve minor differences in the Round and Trim functions
Proposed Solution:
* Define Trim and Round functions in terms of the new implementation
* Resolve all rounding diffs at once
* Reduce merge difficulty with future GIO removal commits
lefticus
added
the
Refactoring
|
Another good piece of this project. I don't have any issues with this work directly, however, examining the diffs made me dig into the eio Surface Details Report which raisesed some questions. From the eio diff file on In this case, the height field changed from 0.31 to 0.30. So I checked the IDF to verify the original values. From the IDF: So, where does the 0.31 (or 0.30) value come from? Likewise for the 3.05 width value? It seems like one came from the window and the other came from the base surface. The OutputDetailsAndExamples.pdf document makes it seem like these should come from the IDF, but perhaps I'm misunderstanding something. @mjwitte @Myoldmopar thoughts? This work is ready. Feel free to open a separate issue if needed to address my questions above. |
|
That wall is parallel to the x plane at y position = 6.096 and the window is at y position = 6.248 so the window is offset from the wall by 0.152 m. So what is the modeling intent here? What if the window were offset 1 m from the wall? The shading surface is just above the top of the window by 0.122 m and connects flush to the wall surface. Regardless, I don't see a 0.31 or 0.3 distance here. |
|
@rraustad That's how a window reveal (setback) is defined. The shading algorithm shades the window with implied solid surfaces the depth of the reveal This shows later in the eio surface list for Zn002:Wall001:Win001, "Reveal" is 0.15. @mitchute For the shading surfaces, the 3.05 and 0.31 are the"~Width" and "~Height". So, the 0.31 comes from 6.096000 - 5.791000 = 0.305 which seems like it should round to 0.31. |
|
@mjwitte thanks for looking at that. It looks like there's some other rounding/trimming going on upstream from where this EIO line is written. See the values for this surface from the debugger below.
I'm not sure how the old |
|
@mitchute Ok, for outputs like this, it's not a huge deal. The calculations for height and width are probably generalized in some way which could introduce tiny numeric differences. It appears that the old method had some checks to transform repeating 9s up to 10. This appears to be what's going on in the one file with an err diff, VariableRefrigerantFlow_FluidTCtrl_HR_5Zone: The input for this is -20 in the idf: So, it seems the stored value, even if it's -19.9999999 should round up to 20.000? |
|
I already know (since I've been working on this for weeks now) that if I adjust it so that 19.999999998576 rounds up to 20.000 it will cause other values to round up and cause different diffs. I can find no consistent way to reproduce what the old system was doing, and have found at least 2 cases that were clear bugs in the rounding of the old system. For the sake of the knowledge here, I'll get the round 19.99999 rounded up to 20.00000 and show you the difference, and see which version we like better. Update Note: The fundamental problem is that the old code would create a string from the float, then round up that string. This... isn't really how floating point math should work. |
|
I'm currently building the "round up a bit more" version and will push it and we can see what things are different. If neither version is satisfactory we'll have to go back to "rounding up of strings" which I strongly do not recommend. This is adding a weird kind of error to floating point computer math which is already error prone. For those who are not familiar with the issues, see this Compiler Explorer example: https://godbolt.org/z/i8z839 |
|
One last note, to address @mjwitte comment: - ** ~~~ ** ... Heat Recovery Minimum Outdoor Dry-Bulb Temperature = -20.000
+ ** ~~~ ** ... Heat Recovery Minimum Outdoor Dry-Bulb Temperature = -19.999The input for this is -20 in the idf: -19.999, when formatted to 0 decimal places, should clearly be rounded to -20 But what if the actual value is -19.9994, when we format it to 3 sigfigs, should it be -19.999 or -20.000? The math would say -19.999, it's more accurate, the human would say -20.000 perhaps? This is the kind of issue I've been chasing. Particularly frustrating with this specific issue, the "Dry Bulb Temperature" is actually using "TrimSigDigits" which shouldn't be rounding at all (but was)! |
|
@lefticus Thanks for the examples. Never would have thought this seemingly simple exercise would be messy. I would agree that -19.9994 should round to -19.999. But I wouldn't expect an input value of 20.0 to be that far away from 20.0. So, maybe the bigger question is whether Given the small number of non-substantive diffs, and the explanation that the 19.999 is produced by |
Problem:
involve minor differences in the Round and Trim functions
Proposed Solution:
Expected Diffs:
Please Review Numeric Diffs Carefully For Any You Have Concern About During PR Review
After this all future GIO removal PR's should be less difficult.
Pull request overview
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.