Rewrite TimeSpanSource, stop DoubleSource returning infinity - #11
Merged
Merged
Conversation
TimeSpanSource did not work. Measured against the built library: new TimeSpanSource() -> -10675199.02:48:05.4775808 on every draw new TimeSpanSource(1h, 2h) -> 00:33:31 | 00:53:19 | 00:23:35 | ... new TimeSpanSource(2h, 1h) -> endless loop The default range overflowed long in maxTicks - minTicks, the two while loops pulled the result out of the requested range, and a reversed range kept multiplying ticks by Random.Next(1, 3). It picks uniformly from the inclusive tick range now, like DateTimeSource, and throws ArgumentOutOfRangeException on a reversed range. Only ranges symmetric around zero worked before, which is what the old test happened to use. new DoubleSource() returned infinity on every draw, because its default range is the whole range of double and the code computed max - min. DoubleSource, FloatSource and DecimalSource share the overflow safe interpolation of NumberSource now, min * (1 - sample) + max * sample. DecimalSource does it in decimal arithmetic instead of going through double and keeps its full precision: 27879346472004256951386422019 instead of 27879346472004300000000000000. RandomIntegerExtensions becomes RandomNumberExtensions and carries both the inclusive integer pick and the continuous one, so NumberSource and the four older sources cannot drift apart. 868 tests per framework. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The last correctness sweep before the v6.0.0 tag. Both fixes change generated sequences, so they have to land inside the break 6.0 already takes.
TimeSpanSource did not work
Measured against the built library, not read from the code:
Three separate defects:
maxTicks - minTicksoverflowslongover the default range, the twowhileloops pull the result out of the range that was asked for, and a reversed range keeps doingticks *= Random.Next(1, 3)forever. Asking for one to two hours and getting 22 to 58 minutes is the most natural way anyone would use the type.It picks uniformly from the inclusive tick range now, the same way
DateTimeSourcedoes, and throwsArgumentOutOfRangeExceptionon a reversed range. Only ranges symmetric around zero worked before, which is exactly what the old stable-sequence test used, so nothing caught it.DoubleSource returned infinity
new DoubleSource()gave∞on every draw: the default range is the whole range ofdoubleand the code computedmax - min.DoubleSource,FloatSourceandDecimalSourcenow share the overflow-safe interpolationmin * (1 - sample) + max * samplethatNumberSource<T>already used.DecimalSourcedoes it in decimal arithmetic instead of the detour throughdouble, so it keeps its full precision:RandomIntegerExtensionsbecomesRandomNumberExtensionsand holds both the inclusive integer pick and the continuous one, soNumberSource<T>and the four older sources cannot drift apart again.Verification
dotnet build -c Releasezero warnings, 868 tests per framework on net8/9/10, 2604 total, all green. The stable sequences ofTimeSpanSourceandFloatSourcechanged and were regenerated; twelve new tests pin the fixed behaviour, including the three cases above.🤖 Generated with Claude Code