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

Diffing_Engine: implementation of NumericalDifferenceInclusion Tolerance approximation to be reviewed #3039

Closed
pawelbaran opened this issue Apr 13, 2023 · 0 comments · Fixed by #3040
Assignees
Labels
type:bug Error or unexpected behaviour

Comments

@pawelbaran
Copy link
Member

pawelbaran commented Apr 13, 2023

Description:

(Edited by @alelom)

Currently, the Diffing uses a numerical comparison mechanism implemented in the NumericalDifferenceInclusion() method. This method has 2 distinct numerical approximation mechanisms that can be invoked for numerical comparisons: Tolerance and SignificantFigures.

The Tolerance approximation mechanism is implemented like so: when two numbers are passed to the method NumericalDifferenceInclusion() method, they are individually rounded down using the specified tolerance, and then compared between each other. If they are found different, the method returns true (i.e. they are seen as different by the Diffing), otherwise false.

double value1 = double.Parse(number1.ToString()).RoundWithTolerance(toleranceToUse);
double value2 = double.Parse(number2.ToString()).RoundWithTolerance(toleranceToUse);
// If, once rounded, the numbers are the same, it means that we do not want to consider this Difference. Skip.
if (value1 == value2)
return false;

This logic is not correct. When an user uses the Tolerance, they expect the difference between the number to be compared against the Tolerance.

Instead, the above logic is correct when using the SignificantFigures approximation. This considers how many significant figures should be accounted for when comparing numbers (use case example: engineers only account for significant figures when comparing large forces).

Proposed changes

When using a numeric tolerance, the logic should instead be: compute the difference between the numbers, then see if it is larger than the specified tolerance.

Test File:

On SharePoint.

@pawelbaran pawelbaran added the type:bug Error or unexpected behaviour label Apr 13, 2023
@alelom alelom transferred this issue from BHoM/Revit_Toolkit Apr 13, 2023
@alelom alelom changed the title Type parameter diffing tolerance bug Diffing_Engine: implementation of Tolerance approximation to be reviewed Apr 13, 2023
@alelom alelom changed the title Diffing_Engine: implementation of Tolerance approximation to be reviewed Diffing_Engine: implementation of NumericalDifferenceInclusion Tolerance approximation to be reviewed Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants