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

Add a FuzzyEq trait #50

Open
cmyr opened this issue Jul 25, 2019 · 5 comments
Open

Add a FuzzyEq trait #50

cmyr opened this issue Jul 25, 2019 · 5 comments
Labels
enhancement New feature or request

Comments

@cmyr
Copy link
Member

cmyr commented Jul 25, 2019

We should have a floating-point aware equality comparison. There are crates out there that provide this logic, and I think it's tricky enough that it's worth a dependency if somebody has done it well.

I suspect the best way to do this would be via some sort of FuzzyEq trait. the float-cmp crate looks like a likely candidate, but I would ideally like something that could, say, infer a good epsilon value for you based on the input values, although maybe this is a bit optimistic.

@richard-uk1
Copy link
Collaborator

richard-uk1 commented Mar 18, 2023

I agree about float-cmp. I have a fork where I started implementing FuzzyEq for the kurbo types. The idea of the float-cmp trait is that it allows you to choose between ULPS and a fixed epsilon. IIUC, ULPS is an integer that represents the number of fp numbers between the two comparison numbers, which means that the absolute range of accepted numbers scales with the size of the (largest) input. I think this gives you what you want @cmyr?

The downside of the comprehensiveness of the FuzzyEq trait is that it is more work to implement, but IMO this is worth it, because being able to test functions for accuracy is so important.

@richard-uk1 richard-uk1 added enhancement New feature or request needs discussion This change requires some more discussion before we decide we definitely want it labels Mar 18, 2023
@cmyr
Copy link
Member Author

cmyr commented Mar 20, 2023

Yea, that sounds about right. I'd be curious to see what the API actually looks like, if you have a branch?

@richard-uk1
Copy link
Collaborator

We talked at the xilem office hours. We decided that the best way forward is to experiment with using float-cmp in tests, but not expose it in the public API. We can then get a feel for how well it works before we commit to it.

@richard-uk1 richard-uk1 removed the needs discussion This change requires some more discussion before we decide we definitely want it label Mar 22, 2023
@ctrlcctrlv
Copy link

Why not expose it with an option like IOF does?

@richard-uk1
Copy link
Collaborator

richard-uk1 commented Mar 22, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants