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

Reduces JET errors #359

Merged
merged 4 commits into from
Sep 14, 2024
Merged

Reduces JET errors #359

merged 4 commits into from
Sep 14, 2024

Conversation

Fe-r-oz
Copy link
Contributor

@Fe-r-oz Fe-r-oz commented Sep 12, 2024

Thanks for the JET simplification. Let's reduce it a bit further to 15 :)

The fast_row and quantum_mallows were not visible locally, so that's why locally it showed 17 but nightly showed it 19.

  • The code is properly formatted and commented.
  • [na] Substantial new functionality is documented within the docs.
  • All new functionality is tested.
  • All of the automated tests on github pass.
  • We recently started enforcing formatting checks. If formatting issues are reported in the new code you have written, please correct them.

@Fe-r-oz Fe-r-oz changed the title Reduces JET errors to 17 Reduces JET errors to 19 Sep 12, 2024
@Fe-r-oz Fe-r-oz changed the title Reduces JET errors to 19 Reduces JET errors to 15 Sep 13, 2024
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Sep 13, 2024

The PR is ready for review, Thank you.

src/sumtypes.jl Outdated
Comment on lines 33 to 37
variant_args = []
for t in _types(type)
push!(variant_args, :(::$t))
end
Expr(:call, _symbol(type), variant_args...)
Copy link
Member

Choose a reason for hiding this comment

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

This does not seem to affect the number of JET warnings in my tests. Why is it needed?

end
end
return join([digits_subchars[d+1] for d in reverse(dlist)])
end
Copy link
Member

Choose a reason for hiding this comment

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

this does not seem to affect the number of jet warnings for me

Comment on lines 766 to 768
@inline comm!(v, l::PauliOperator, r::Tableau, i::Int) = comm!(v, l, r, i)
@inline comm!(v, l::Tableau, r::PauliOperator, i::Int) = comm!(v, l, r, i)
@inline comm!(v, s::Tableau, l::Int, r::Int) = comm!(v, s, l, r)
Copy link
Member

Choose a reason for hiding this comment

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

These cause stack overflow errors as they are referring to themselves:

julia> comm!([1,2,3], T"X", P"X", 1)
ERROR: StackOverflowError:
Stacktrace:
 [1] comm!(v::Vector{…}, l::QuantumClifford.Tableau{…}, r::PauliOperator{…}, i::Int64) (repeats 79984 times)
   @ QuantumClifford ~/Documents/ScratchSpace/quantumjulia/QuantumClifford.jl/src/QuantumClifford.jl:767
Some type information was truncated. Use `show(err)` to see complete types.

Copy link
Member

Choose a reason for hiding this comment

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

they would need tests

@Krastanov Krastanov added the Skip-Changelog label for control of CI: skips the changelog check label Sep 14, 2024
@Krastanov Krastanov changed the title Reduces JET errors to 15 Reduces JET errors Sep 14, 2024
@Krastanov Krastanov merged commit 3616933 into QuantumSavory:master Sep 14, 2024
14 of 16 checks passed
@Fe-r-oz Fe-r-oz deleted the solve branch September 14, 2024 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip-Changelog label for control of CI: skips the changelog check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants