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

Aqua static analysis [$200] #135

Closed
Krastanov opened this issue Jul 9, 2024 · 9 comments · Fixed by #143
Closed

Aqua static analysis [$200] #135

Krastanov opened this issue Jul 9, 2024 · 9 comments · Fixed by #143
Assignees
Labels
bounty:reserved bounty:200 bug bounty There is an award for solving this issue.

Comments

@Krastanov
Copy link
Member

Bug bounty logistic details (click to expand)

To claim exclusive time to work on this bounty either post a comment here or message [email protected] with:

  • your name
  • github username
  • (optional) a brief list of previous pertinent projects you have engaged in

Currently the project is claimed by no one until ....

If you want to, you can work on this project without making a claim, however claims are encouraged to give you and other contributors peace of mind. Whoever has made a claim takes precedence when solutions are considered.

You can always propose your own funded project, if you would like to contribute something of value that is not yet covered by an official bounty.

Project: Aqua static analysis [$200]

We run Aqua.jl static analysis during CI for this library, but currently, we have disabled some checks, as they raise too many errors. This bounty is awarded for fixing these issues and enabling these checks. The bounty might require upstreaming some work from here into QuantumInterface.jl or other packages.

Required skills: Generic Julia skills.

Reviewer: Stefan Krastanov

Duration: 1 month

Payout procedure:

The Funding for these bounties comes from the National Science Foundation and from the NSF Center for Quantum Networks. The payouts are managed by the NumFOCUS foundation and processed in bulk once every two months. If you live in a country in which NumFOCUS can make payments, you can participate in this bounty program.

Click here for more details about the bug bounty program.

@Krastanov Krastanov added bug bounty There is an award for solving this issue. bounty:200 labels Jul 9, 2024
@Krastanov Krastanov transferred this issue from QuantumSavory/QuantumClifford.jl Jul 12, 2024
@Tortar
Copy link
Contributor

Tortar commented Aug 1, 2024

Hi @Krastanov, I'd like to take this.

From my experience, ambiguities are not so easy to fix because they can stem from any depending package and be a lot. But I will try to go through them, in one of my packages I took the route to only test the direct ones anyway (https://github.com/JuliaDynamics/Agents.jl/blob/main/test/package_sanity_tests.jl)

@Krastanov
Copy link
Member Author

Hi, @Tortar ! That sounds reasonable. Thanks for taking this on!

@Tortar
Copy link
Contributor

Tortar commented Aug 2, 2024

thanks @Krastanov I almost fixed all types piracies except two which elude me: the two apply! methods at https://github.com/QuantumSavory/QuantumSavory.jl/blob/master/src/representations.jl pirate Symbolic because apply! comes from QuantumInterface and Symbolic from SymbolicUtils. Also, the default_repr comes from QuantumSavory, express and UseAsOperation from QuantumSymbolics. So it seems a bit of a mess to solve the piracy to me. Do you have any idea on how to approach these ones?

@Tortar
Copy link
Contributor

Tortar commented Aug 2, 2024

I will put piracies=(;treat_as_own=[QuantumSavory.SymbolicUtils.Symbolic]) for now

@Krastanov
Copy link
Member Author

Yeah, these two piracies are unavoidable until significant other work happens, but we should probably guard against more of them happening. Is it possible to check specifically that exactly two piracies are happening and to add a separate @test_broken for the fact the piracies are not zero?

I did not know of treat_as_own, thanks for sharing!

@Tortar
Copy link
Contributor

Tortar commented Aug 3, 2024

Yes probably we could add an ad-hoc Aqua tests about piracies which checks the number of piracies in the package...I have to say that the PR I opened now added 4 more piracies related to the Symbolic + apply! combination to solve some ambiguities within the package, they have the same cause of the others so I think it is legit to add them, so I will add an Aqua tests which will print when tests are run that there are still six piracies

@Tortar
Copy link
Contributor

Tortar commented Aug 4, 2024

Actually we can probably solve all these piracies by moving apply! from QuantumInterface to QuantumSavory, do you see any problems doing that? I think it will also simplify some other things. Seems possible because actually apply! has zero methods and acquires methods only when extended in QuantumSavory

@Krastanov
Copy link
Member Author

apply! is also used by other packages that do not depend on QuantumSavory, e.g. QuantumOptics, QuantumClifford, and QuantumSymbolics. I do not think we can move it easily (admittedly some of these packages are coupled in unpleasant ways, to an extend because things have evolved pretty organically without having the piracy tests turned on). I recognize it is a bit of a mess (hence the bounty ;)

@Krastanov
Copy link
Member Author

This diagram kinda describes the relationship
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty:reserved bounty:200 bug bounty There is an award for solving this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants