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

Should we implement @range on float and double shapes? #2007

Open
Tracked by #1401
LukeMathWalker opened this issue Nov 18, 2022 · 3 comments
Open
Tracked by #1401

Should we implement @range on float and double shapes? #2007

LukeMathWalker opened this issue Nov 18, 2022 · 3 comments

Comments

@LukeMathWalker
Copy link
Contributor

The smithy spec seems to suggest that @range is allowed on float and double shapes.

We should clarify if that is correct and how it should be implemented (e.g. should range allow to specify a tolerance threshold for comparisons?).

@crisidev
Copy link
Contributor

Shouldn't this issue be in the awslabs/smithy repo? The spec "seems to suggest", but it not really clear right?

@LukeMathWalker
Copy link
Contributor Author

LukeMathWalker commented Nov 21, 2022

Yes, this is for tracking on our side/to give visibility to users of smithy-rs who might be interested.
@david-perez is going to follow up with the Smithy maintainers to get clarity (and if we have a public issue we'll link it here).

@david-perez
Copy link
Contributor

david-perez commented May 25, 2023

I don't want to implement @range on floating point shapes because comparing floating point numbers without a tolerance threshold is almost always not what you want.

Consider a client performing floating point arithmetic and sending the result to a service:

$ python3 -c "print((0.1 + 0.2) > 0.3)"
True

Having the server framework compare floating point numbers generically can have devastating consequences in certain application domains. Depending on the hardware, FPU, compiler and compiler flags, an operation like 0.1 * 10 may yield different results, and as such may or may not satisfy @range(min: 1) in all target language implementations of Smithy.

This is not even getting into the problems that arise from negative and positive infinities, NaN breaking total ordering (the reason why Rust only implements PartialOrd and not Ord), or the total ordering imposed by IEEE-754.

I think that there's no easy way for Smithy to support float comparisons as-is interoperably across languages and architectures without prescribing an epsilon threshold.

This is also one of the reasons why Smithy dropped support for floating point shapes from set shapes (which ultimately got deprecated in favor of @uniqueItems in IDL v2), and the sole reason why @uniqueItems cannot apply to a list shape that transitively contains floats, doubles, or documents. Unfortunately, there's currently several uses of @range as-is on floating point shapes in existing AWS models that Smithy has limited options to address this.

However, smithy-rs, especially server-side development, is usually greenfield, and so we have a chance here to warn service owners. I therefore think it's best that this is not implemented, and instead each smithy-rs application, depending on their needs, enforce the check themselves as they require.

david-perez added a commit that referenced this issue May 25, 2023
david-perez added a commit that referenced this issue May 25, 2023
To point to a better issue
#2007.

Also let users know this is unlikely to ever get implemented.
github-merge-queue bot pushed a commit that referenced this issue Jul 21, 2023
#2727)

To point to a better issue
#2007.

Also let users know this is unlikely to ever get implemented.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
@jdisanti jdisanti added the server Rust server SDK label Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants