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.

Sign up for GitHub

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

Jump to bottom

P1673R6 changes #186

Closed
wants to merge 8 commits into from
Closed

P1673R6 changes #186

wants to merge 8 commits into from

Conversation

mhoemmen
Copy link
Contributor

Changes for P1673R6.

Mark Hoemmen added 5 commits December 14, 2021 16:12
P0009: Revision 5 changes
15b1473 
1. Adopt P0009R13, which changes mdspan::operator[] to operator().

2. Update references to P0009.
P1673R6: Update references to P0009 and P2128
2be58f3
P1673R6: Apply rank_2k fix from PR
9771f82 
Apply fix from the following PR:

ORNL#184
P1673R6: Remove refs to rank > 2
fc889b0 
Remove references to any mdspan rank greater than 2.
(These were left over from earlier versions of the proposal
that included "batched" operations.)

This should fix ORNL#143.
P1673R6: Fix vector_sum_of_squares in table
e316c5b 
Change sum_of_squares to vector_sum_of_squares in comparison table.
This should fix kokkos/stdBLAS#98 .
@mhoemmen mhoemmen mentioned this pull request Dec 15, 2021
Closed
P1673R6: Requires -> Preconditions:
75f08bf 
Replace "Requires" with "Preconditions," per new wording guidelines.
This fixes kokkos/stdBLAS#95 .
P1673R6: Fix rank_k_update ambiguity
b875ed9 
Remove all overloads of `symmetric_matrix_rank_k_update` and
`hermitian_matrix_rank_k_update` that do not take an `alpha` parameter.
This prevents ambiguity between overloads
that take `ExecutionPolicy&&` but not `alpha`,
and overloads that take `alpha` but not `ExecutionPolicy&&`.

This fixes kokkos/stdBLAS#134 .
@mhoemmen mhoemmen mentioned this pull request Dec 15, 2021
Closed
P1673R6: Harmonize conjugated_scalar w/ impl
f13bd4a 
Harmonize with the implementation, by adding `operator+`, `operator*`,
and comparison operators to `conjugated_scalar`.
@crtrott
Copy link
Collaborator

crtrott commented Dec 15, 2021

I don't think we should remove the overloads without alpha, its not ambiguous at all if you use the correct requires clauses.

crtrott
crtrott requested changes Dec 15, 2021
View reviewed changes
Copy link
Collaborator

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

I don't think we should remove the overloads - at least not without talking to all the other authors. There is no ambiguity here if you use the correct preconditions (i.e. C++20 requires clauses).

@mhoemmen
Copy link
Contributor Author

I don't think we should remove the overloads - at least not without talking to all the other authors. There is no ambiguity here if you use the correct preconditions (i.e. C++20 requires clauses).

I'll put them back in the next version. That was a bit of a hasty decision. On the other hand, the ambiguity would be challenging to resolve without some complicated requires clauses, and I'm not sure if LEWG would appreciate the lack of precedent.

In any case, I submitted this as R6, so please do merge it. I can file an issue to bring back the overloads, if we can figure out how to constrain them correctly.

@mhoemmen mhoemmen mentioned this pull request Dec 15, 2021
Open
@mhoemmen mhoemmen mentioned this pull request Apr 15, 2022
Merged
@mhoemmen
Copy link
Contributor Author

Superseded by PR #222 . Thanks!

@mhoemmen mhoemmen closed this Apr 15, 2022
@mhoemmen
Copy link
Contributor Author

Merge of #222 means we can delete this branch.

@mhoemmen mhoemmen deleted the P1673R6 branch April 27, 2022 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crtrott crtrott crtrott requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mhoemmen @crtrott