QuantumSavory / QuantumSavory.jl

A full stack simulator of quantum hardware, from the low-level analog physics to high-level network dynamics. Includes discrete event simulator, symbolic representation for quantum object, and works with many backend simulators.
https://quantumsavory.github.io/QuantumSavory.jl/
MIT License
30 stars 11 forks source link

Aqua static analysis [$200] #135

Closed Krastanov closed 3 weeks ago

Krastanov commented 2 months ago
Bug bounty logistic details (click to expand) To claim exclusive time to work on this bounty either post a comment here or message [skrastanov@umass.edu](mailto:skrastanov@umass.edu) 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.

Tortar commented 1 month ago

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 commented 1 month ago

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

Tortar commented 1 month ago

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 commented 1 month ago

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

Krastanov commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

This diagram kinda describes the relationship image