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.

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

Bring conjugated_accessor and scaled_accessor in line with the C++ Working Draft #260

Merged
merged 16 commits into from
Sep 27, 2024

Conversation

mhoemmen
Copy link
Contributor

@mhoemmen mhoemmen commented Jul 26, 2023

  • Rename accessor_scaled to scaled_accessor
  • Change scaled_accessor::reference to the actual product value, and change element_type accordingly
  • Add full converting constructor (scaling factor and/or input scaled_accessor's nested accessor could differ)
  • Make scaled_accessor::access conditionally (instead of unconditionally) noexcept
  • Rename accessor_conjugate to conjugate_accessor
  • Implement analogous changes to conjugate_accessor
  • Fix implementation of conj-if-needed, and add implementations (and tests) of real-if-needed and imag-if-needed
  • Add tests mixing result of scaled with an mdspan whose accessor uses a proxy reference as its reference type. This increases confidence that the new (proxy-reference-free) design works.

accessor_scaled::access now returns element_type,
not a proxy reference type.  The product is thus
evaluated immediately on access.
* Rename accessor_conjugate to conjugate accessor
* Don't use proxy reference machinery; instead,
  just make the "reference" a value
* Add missing converting constructor
...into their own reusable header file.
Move conj_if_needed, real_part, and imag_part tests
out of proxy_refs.cpp into a new test file.
@mhoemmen mhoemmen changed the title Implement scaled_accessor wording changes from LWG review Implement scaled_accessor LWG review changes (& analogous conjugate_accessor changes) Jul 26, 2023
This better reflects the header's contents.

Also, fix the kokkos-kernels implementation, as it no longer needs
the is_complex trait to determine if conj should be called
on a number type.
scaled_accessor and conjugate_accessor no longer use
proxy reference types, so we don't need to include
that header in linalg any more.  We still have a test
for the proxy reference types, so move proxy_reference.hpp
to the test directory.
@mhoemmen mhoemmen changed the title Implement scaled_accessor LWG review changes (& analogous conjugate_accessor changes) Bring conjugated_accessor and scaled_accessor in line with the C++ Working Draft Sep 27, 2024
* Rename accessor_scaled to scaled_accessor, and
  accessor_conjugate to conjugated_accessor

* Remove proxy references from both accessor types

* Add missing conditional explicit to both accessors'
  constructors

* Otherwise bring conjugated_accessor and scaled_accessor
  in line with the Standard

* Fix namespaces where needed

* Fix implementation of conj-if-needed, and add implementations (and
  tests) of real-if-needed and imag-if-needed

* Add tests mixing result of scaled with an mdspan whose accessor
  uses a proxy reference as its reference type.  This increases
  confidence that the proxy-reference-free design works.
@crtrott
Copy link
Member

crtrott commented Sep 27, 2024

I counter checked with standard, and compiled and ran tests.

@crtrott crtrott merged commit 66ff8e7 into kokkos:main Sep 27, 2024
3 checks passed
@mhoemmen mhoemmen deleted the remove-proxy-reference branch September 27, 2024 23:43
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