XanaduAI / thewalrus

A library for the calculation of hafnians, Hermite polynomials and Gaussian boson sampling.
https://the-walrus.readthedocs.io
Apache License 2.0
100 stars 55 forks source link

Port CI to github actions #292

Closed sduquemesa closed 2 years ago

sduquemesa commented 2 years ago

Context: It might make sense to port the wheel building CI away from a combination of CircleCI/Appveyor, for a couple of reasons:

In addition, we could also set up continuous deployment; when a new GitHub release is made, the wheels could be automatically built, tested, and uploaded.

Description of the Change:

Benefits:

Possible Drawbacks: Performance -- GitHub actions might be slower than the current combination of CircleCI/Appveyor.

Related GitHub Issues: Closes #191

josh146 commented 2 years ago

@sduquemesa this is great, thanks for tackling this!

sduquemesa commented 2 years ago

Hi @josh146 and @thisac, I believe this is ready for a first round of reviews!

Currently wheels are built for

as done before by CircleCI+Appveyor. Any arch missing?

Should I set up a continuous deployment actions as well; i.e., automatic upload of wheels on a release?

sduquemesa commented 2 years ago

Formatting issues are addressed on PR #291

sduquemesa commented 2 years ago

CircleCI and Appvyor fail because their configuration files were removed from this branch.

josh146 commented 2 years ago

One question @sduquemesa, I can't seem to find the artifacts created by the wheel builders? image

sduquemesa commented 2 years ago

One question @sduquemesa, I can't seem to find the artifacts created by the wheel builders? image

@josh146, wheels were only uploaded on master; I have removed that condition with this commit. Now you should see the uploaded artifacts 🚀

josh146 commented 2 years ago

wheels were only uploaded on master

Thanks @sduquemesa for clarifying!

I see them! Although, isn't having them only being built on master better? 🤔

I agree @thisac --- although it was my confusion that caused this, I think the original behaviour you had @sduquemesa was the correct one

sduquemesa commented 2 years ago

Thanks @thisac and @josh146 for your comments! I think I have integrated the suggested changes. This is ready to go if there are no more comments 🚀 To merge @josh146 will have to override Circle and Appveyor tests.