Add Rounding to Fix Numeric Error in LinearLeastSquaresFit#22
Open
jhazentia wants to merge 1 commit intoNTIA:masterfrom
Open
Add Rounding to Fix Numeric Error in LinearLeastSquaresFit#22jhazentia wants to merge 1 commit intoNTIA:masterfrom
jhazentia wants to merge 1 commit intoNTIA:masterfrom
Conversation
aromanielloNTIA
approved these changes
Nov 21, 2024
Contributor
aromanielloNTIA
left a comment
aromanielloNTIA
left a comment
There was a problem hiding this comment.
Thanks for identifying and fixing this issue. Looks good!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #21
This issue is fixed by adding rounding to fix the numeric error in LinearLeastSquaresFit.
Examples of numeric error and rounding correction in LinearLeastSquaresFit are below
(values observed debugging in // comments). The examples include "_r" variables
to observe the rounding fix.
Case 1:
Windows PFL:
d_start / pfl[1] // 1490.9999999999491
int(d_start / pfl[1]) // 1490
int i_start = int(fdim(d_start / pfl[1], 0.0)); // 1490
int i_start_r = int(round(fdim(d_start / pfl[1], 0.0))); // 1491
d_end / pfl[1] // 1498.9999999999945
int(np - (np - (d_end / pfl[1]))) // 1498
int i_end = np - int(fdim(np, d_end / pfl[1])); // 1499
int i_end_r = np - int(round(fdim(np, d_end / pfl[1]))); // 1499
Linux PFL:
d_start / pfl[1] // 1491.0000000000146
int(d_start / pfl[1]) // 1491
int i_start = int(fdim(d_start / pfl[1], 0.0)); // 1491
int i_start_r = int(round(fdim(d_start / pfl[1], 0.0))); // 1491
d_end / pfl[1] // 1499.0000000000018
int(np - (np - (d_end / pfl[1]))) // 1499
int i_end = np - int(fdim(np, d_end / pfl[1])); // 1500
int i_end_r = np - int(round(fdim(np, d_end / pfl[1]))); // 1499
Case 2:
Windows PFL:
d_start / pfl[1] // 1056.9999999999991
int(d_start / pfl[1]) // 1056
int i_start = int(fdim(d_start / pfl[1], 0.0)); // 1056
int i_start_r = int(round(fdim(d_start / pfl[1], 0.0))); // 1057
d_end / pfl[1] // 1064.9999999999998
int(np - (np - (d_end / pfl[1]))) // 1064
int i_end = np - int(fdim(np, d_end / pfl[1])); // 1065
int i_end_r = np - int(round(fdim(np, d_end / pfl[1]))); // 1065
Linux PFL:
d_start / pfl[1] // 1057.0000000000084
int(d_start / pfl[1]) // 1057
int i_start = int(fdim(d_start / pfl[1], 0.0)); // 1057
int i_start_r = int(round(fdim(d_start / pfl[1], 0.0))); // 1057
d_end / pfl[1] // 1065.0000000000009
int(np - (np - (d_end / pfl[1]))) // 1065
int i_end = np - int(fdim(np, d_end / pfl[1])); // 1066
int i_end_r = np - int(round(fdim(np, d_end / pfl[1]))); // 1065
Here are output files after applying the fix for the 2 cases in original issue, showing matching results:
case_1_itm_command_output_linux_pfl.txt
case_1_itm_command_output_windows_pfl.txt
case_2_itm_command_output_linux_pfl.txt
case_2_itm_command_output_windows_pfl.txt