Skip to content

Implement 2D Dot Product#7

Open
TimTheBig wants to merge 3 commits into
timschmidt:mainfrom
TimTheBig:more_fns
Open

Implement 2D Dot Product#7
TimTheBig wants to merge 3 commits into
timschmidt:mainfrom
TimTheBig:more_fns

Conversation

@TimTheBig
Copy link
Copy Markdown

This patch adds dot2_refs, with tests and benches

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements two-lane dot product functions (dot2_refs and active_dot2_refs) for the Real type, along with associated microbenchmarks and unit tests. The implementation includes optimized paths for rational numbers and symbolic fallbacks. Feedback focuses on issues within benchmarks.md, specifically the introduction of redundant rows for existing benchmarks, the accidental clearing of previous timing data for atanh special forms, and the inclusion of atan2 benchmark results that appear to be unrelated to the current changes.

Comment thread benchmarks.md Outdated
Comment thread benchmarks.md Outdated
Comment thread benchmarks.md Outdated
Copy link
Copy Markdown
Author

@TimTheBig TimTheBig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes review comments

Comment thread benchmarks.md Outdated
Comment thread benchmarks.md Outdated
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Ryan <132001783+TimTheBig@users.noreply.github.com>
Comment thread src/real/tests.rs Outdated
@TimTheBig
Copy link
Copy Markdown
Author

@timschmidt the review comments have been fixed it's good to go, do you mind having a quick look?

@danmodus
Copy link
Copy Markdown

Are any plans to merge this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants