Skip to content

ARROW-6311: [Java] Make ApproxEqualsVisitor accept DiffFunction to make it more flexible#5155

Closed
tianchen92 wants to merge 1 commit intoapache:masterfrom
tianchen92:ARROW-6311
Closed

ARROW-6311: [Java] Make ApproxEqualsVisitor accept DiffFunction to make it more flexible#5155
tianchen92 wants to merge 1 commit intoapache:masterfrom
tianchen92:ARROW-6311

Conversation

@tianchen92
Copy link
Copy Markdown
Contributor

Related to ARROW-6311.

Currently ApproxEqualsVisitor will accept a epsilon for both float and double compare, and the difference calculation is always Math.abs(f1-f2)

For some cases like Validator it is not very suitable as:
i. it has different epsilon values for float/double
ii. it difference function is not Math.abs(f1-f2)

To resolve these, make this visitor accept both float/double epsilons and diff functions.

@tianchen92
Copy link
Copy Markdown
Contributor Author

cc @pravindra

Copy link
Copy Markdown
Contributor

@pravindra pravindra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change lgtm. some minor questions

  • is it useful to have different epsilons for floats and doubles (i guess a double epsilon can cover both)?
  • did you consider using a Java.Util.Comparator instead of the diff functions ?

@tianchen92
Copy link
Copy Markdown
Contributor Author

the change lgtm. some minor questions

  • is it useful to have different epsilons for floats and doubles (i guess a double epsilon can cover both)?
  • did you consider using a Java.Util.Comparator instead of the diff functions ?

Thanks for your comments,
for 1, most cases an epsilon can cover both, but in some cases they may be different, such as Validator.java(1.0E-6f and 1.0E-12f for each). And we have different constructor(with one epsilon for both or have two epsilon) for different cases.
for 2, the diff functions are used to calculate differences between values like Math.abs(f1-f2) by default or something as below in Validator.java, and maybe other implementations.

float average = Math.abs((f1 + f2) / 2);
float differenceScaled = Math.abs(f1 - f2) / (average == 0.0f ? 1f : average);

java.util.comparator seems only compare two values which is larger i think can’t handle this?
Thanks!

@pravindra pravindra closed this in 4c59aad Aug 23, 2019
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
…ke it more flexible

Related to [ARROW-6311](https://issues.apache.org/jira/browse/ARROW-6311).

Currently ApproxEqualsVisitor will accept a epsilon for both float and double compare, and the difference calculation is always Math.abs(f1-f2)

For some cases like Validator it is not very suitable as:
i. it has different epsilon values for float/double
ii. it difference function is not Math.abs(f1-f2)

To resolve these, make this visitor accept both float/double epsilons and diff functions.

Closes apache#5155 from tianchen92/ARROW-6311 and squashes the following commits:

41a5dba <tianchen> ARROW-6311:  Make ApproxEqualsVisitor accept DiffFunction to make it more flexible

Authored-by: tianchen <niki.lj@alibaba-inc.com>
Signed-off-by: Pindikura Ravindra <ravindra@dremio.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants