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

fix semantics of eltype on TracedRNumber #287

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mofeing
Copy link
Collaborator

@mofeing mofeing commented Nov 18, 2024

Base.eltype on TracedRNumber is semantically wrong because TracedRNumber, as a Number, should return its own type, not the interior type. For example,

julia> eltype(ComplexF64)
ComplexF64 (alias for Complex{Float64})

julia> eltype(Reactant.TracedRNumber{ComplexF64})
ComplexF64 (alias for Complex{Float64})

This behavior is giving some problems when using Yao to get the matrix representation of a parametrized quantum gate when the parameter is traced using TracedRNumber.

julia> using Reactant, YaoBlocks

julia> circ = chain(1, put(1 => H), put(1 => Rz(p)))
nqubits: 1
chain
├─ put on (1)
│  └─ H
└─ put on (1)
   └─ rot(Z, TracedRNumber{ComplexF64}(()))

# before
julia> parameters_eltype(circ)
ComplexF64 (alias for Complex{Float64})

# after
julia> parameters_eltype(circ)
Reactant.TracedRNumber{ComplexF64}

@mofeing mofeing requested a review from avik-pal November 18, 2024 21:51
@mofeing
Copy link
Collaborator Author

mofeing commented Nov 18, 2024

seems like the error is somewhere else? did we broke sth @wsmoses?

@mofeing
Copy link
Collaborator Author

mofeing commented Nov 19, 2024

@avik-pal any idea of why this breaks so many tests?

@wsmoses
Copy link
Member

wsmoses commented Nov 19, 2024

Not sure but probably other parts of the code need to be rewritten to use comcreteeltype

@mofeing
Copy link
Collaborator Author

mofeing commented Nov 27, 2024

in today's meeting and following what I wrote in #308 (comment), I see 3 different eltype implementations:

  • traced_eltype: return TracedRNumber{T} on a TracedRArray{T}. otherwise, call eltype.
  • concrete_eltype: return ConcreteRNumber{T} on a ConcreteRArray{T}. otherwise, call eltype.
  • regular Base.eltype: return T from RArray{T}

in general regular eltype should be used to fix problems with idiomatic Julia libraries, but the definition of eltype on an array is that the type of calling iterate on the array should be returned (e.g. calling x[1] where x isa ConcreteRArray{Float64,1} returns a Float64, not a ConcreteRNumber{Float64}, and the same should be expected from TracedRArray)

once some consensus is achieved and this PR is merged, some tests for Yao should be written and tested without current pkg extension

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.

3 participants