fix: performance of erfinv #7568 (from dotnet/machinelearning#7569)#51
Draft
Copilot wants to merge 1 commit into
Draft
fix: performance of erfinv #7568 (from dotnet/machinelearning#7569)#51Copilot wants to merge 1 commit into
Copilot wants to merge 1 commit into
Conversation
…from dotnet#7569) Agent-Logs-Url: https://github.com/JanKrivanek/machinelearning/sessions/47b47f1f-d882-4f96-85b8-042a37ad3206 Co-authored-by: JanKrivanek <3809076+JanKrivanek@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
JanKrivanek
April 7, 2026 15:59
View session
Owner
|
/add-tests |
|
❌ Add Tests for PR Changes failed. Please review the logs for details. |
Owner
|
/add-tests |
|
❌ Add Tests for PR Changes failed. Please review the logs for details. |
Owner
|
/add-tests |
|
✅ Add Tests for PR Changes completed successfully! |
|
Pull request created: #53
|
Owner
|
/add-tests |
|
✅ Add Tests for PR Changes completed successfully! |
github-actions Bot
added a commit
that referenced
this pull request
Apr 8, 2026
Add 57 unit tests covering the ProbabilityFunctions class changed in PR #51. Tests cover: - Erfinv boundary/special values (NaN, ±∞, 0) - Erfinv consistency (coefficient caching returns same result) - Erf/Erfinv round-trip accuracy - Erf/Erfc special values and symmetry - Erf + Erfc = 1 complement property - Probit valid/invalid inputs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #54
|
This was referenced Apr 8, 2026
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.
Summary
Copies the changes from dotnet#7569 (by @JeWaVe) into this fork.
Fixes: dotnet#7568
Changes
Before: An array of 1000 doubles is allocated and computed for each
Erfinvcall.Now: Lazy allocation for coefficients in a
readonly structwith a static constructor. Allocation and computation is done only once, the first timeErfinvis invoked.Changed file
src/Microsoft.ML.CpuMath/ProbabilityFunctions.cs— Moved the series coefficient computation into a newErfInvSeriesCoefficientsstruct with a static constructor, so the coefficients are computed once and reused across all calls toErfinv.