Make double assertions compare with tolerance instead of precision #6923
Conversation
This file contains 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
Precision might cause small differences to round to a different number. Instead compare with a tolerance which is not sensitive to rounding.
ghost
assigned 
michaelgsharp
approved these changes
Dec 22, 2023
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6923 +/- ##
=======================================
Coverage 68.79% 68.80%
=======================================
Files 1249 1249
Lines 249431 249433 +2
Branches 25510 25510
=======================================
+ Hits 171604 171628 +24
+ Misses 71214 71191 -23
- Partials 6613 6614 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ericstj
added a commit
that referenced
this pull request
Jan 5, 2024
…6923) Precision might cause small differences to round to a different number. Instead compare with a tolerance which is not sensitive to rounding.
michaelgsharp
added a commit
that referenced
this pull request
Jan 9, 2024
* Update dependencies from https://github.com/dotnet/arcade build 20231220.2 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.SignTool , Microsoft.DotNet.SwaggerGenerator.MSBuild , Microsoft.DotNet.XUnitExtensions From Version 8.0.0-beta.23265.1 -> To Version 8.0.0-beta.23620.2 * Fixed version update breaks. * Update XUnitVersion * Update MicrosoftMLOnnxRuntimeVersion to 1.16.3 * Rollback OnnxRuntime and suppress warning * Update to Xunit with fix for xunit/xunit#2821 * Update Centos docker containers * Fix packaging step * Try including stdint.h to fix missing uint8_t on centos * Update Centos test queue * Attempt to use runtime centos-stream8-helix container for tests * Use centos-stream8-mlnet-helix container for testing * Undo changes to test data * Make NETFRAMEWORK ifdef versionless * Only use semi-colons for NoWarn * Fix assert by only accessing idx (#6924) Asserting on `_rowCount < Utils.Size(_valueBoundaries)` was catching a case where `_rowCount`'s update was reordered before `_valueBoundaries` This was unnecessary, since this method doesn't need to use `_rowCount`. Instead, make the asserts use only `idx` which will be maintained consistent with the waiter logic in this cache. �Ensure we only ever use `_rowCount` from the caching thread, so write reordering won't matter. * Don't include the SDK in our helix payload (#6918) * Don't include the SDK in our helix payload I noticed that the tests included the latest SDK - including the host - in our helix payloads. This is a large amount of unnecessary downloads and it also makes it so we use the latest host on the older frameworks which can fail when the latest host drops support for distros. Since our tests shouldn't need the full CLI, remove this from our helix payloads. We'll instead get just the runtime we need through `AdditionalDotNetPackage` * Place Helix downloaded runtime on the PATH Helix only sets the path when the CLI is included, however we don't need the CLI. * Make double assertions compare with tolerance instead of precision (#6923) Precision might cause small differences to round to a different number. Instead compare with a tolerance which is not sensitive to rounding. --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Michael Sharp <misharp@microsoft.com> Co-authored-by: Eric StJohn <ericstj@microsoft.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Precision might cause small differences to round to a different number. Instead compare with a tolerance which is not sensitive to rounding.
See comment here: #6703 (comment)