sandialabs / pyGSTi

A python implementation of Gate Set Tomography
http://www.pygsti.info
Apache License 2.0
137 stars 55 forks source link

Developer infrastructure: mark code with level of user-exposure #354

Open rileyjmurray opened 1 year ago

rileyjmurray commented 1 year ago

pyGSTi is both research software and a tool that's used in "production" QCVV. Its research flavor makes it impractical for us to spend a lot of effort differentiating between a public and a private API. At the same, its role in production code means we need to be judicious about changing pyGSTi in a way that would break users' workflows. These factors combined make it hard for newcomers (like myself) to contribute to pyGSTi. Here's a proposal to help get new contributors un-stuck.

Proposed policy

I suggest the following taxonomy for marking a piece of code's level of "user exposure."

I think everyone is on board with a taxonomy along those lines. However, we didn't discuss how these labels would be used to affect pyGSTi's development. I suggest we use it in the following way.

For now, I'd take "pyGSTi maintainer" to mean @sserita, @coreyostrove, or @enielse.

Note: In the call, we joked a bit about marking certain code as "godforsaken," to signify that we'd like to change it substantially, but we don't quite know how or when to do so. While I think that would be helpful, I don't think we need to figure that out now.

Proposed adoption plan

Over the coming months, the pyGSTi maintainers mark all "high exposure" code. Low- and minimal-exposure code is marked along the way, to the extent possible.

The adoption process will effectively complete when we're comfortable with implicitly designating unlabeled code as "minimal exposure." API changes to any such code only require the approval of one maintainer, but the approving maintainer has a responsibility to think about whether or not the code actually has minimal user exposure.

Request to pyGSTi developers

Comment below indicating if you agree with this general direction, or if you'd like changes. Once we converge on what we want to do, we can figure out how exactly we'll do it.

rileyjmurray commented 1 year ago

Here are some notes from our October 17 meeting.

Everyone's on board with the proposal I made above. (Modulo one edit I had to make.)

We'll implement tags concretely with one-line comments before any def to indicate high, low, or minimal exposure.

It's important to note that these markings are kind of our guess at what the community is using. Their shelf-life is limited.

Adding these kinds of tags is the first phase of a three-phase effort. The second phase is to replace comments with things that are more permanent or functional (like prepending an underscore to the names of minimal-exposure functions). The third phase will be to public API. We have no goals for when we start phase two.

We should keep the community appraised of this effort as we go along. We can have an "API Stability" page of the pyGSTi website that explains the purpose of the tags, solicits user input, and gives advance notice on bigger changes we might make.

CC @sserita, @enielse, @coreyostrove

coreyostrove commented 1 year ago

Excellent work and summary, @rileyjmurray! I am onboard with all of the above.

One minor addendum to the summary. We also discussed integrating the maintenance of this API tagging into our standard PR workflow as part of an update to our contributor guidelines (much like we are effectively handling unit testing additions and documentation presently).

sserita commented 1 year ago

Sounds good to me. Thanks @rileyjmurray for the great work and I agree with @coreyostrove's addendum - really this will also come down to us as reviewers to ensure that any changes to exposure-levels of the code and updating the tags accordingly. This will be most relevant in the second stage where we are working to shift code up and down levels as we work towards some more permanent organization structure I think.

coreyostrove commented 12 months ago

Quick heads up for anyone working on the branch related this this github issue that I temporarily disabled the on-push server-side unit tests for this branch. We'll be making a lot of comment-only pushes as part of these updates, so I figured it wasn't a great use of github runner allocations to be testing against it constantly.

rileyjmurray commented 12 months ago

Good catch!

On Tue, Nov 28, 2023 at 9:53 PM coreyostrove @.***> wrote:

Quick heads up for anyone working on the branch related this this github issue that I temporarily disabled the on-push server-side unit tests for this branch. We'll be making a lot of comment-only pushes as part of these updates, so I figured it wasn't a great use of github runner allocations to be testing against it constantly.

— Reply to this email directly, view it on GitHub https://github.com/pyGSTio/pyGSTi/issues/354#issuecomment-1831131063, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACRLIFBHNDITREEAZASMADDYG2PQ3AVCNFSM6AAAAAA54RXZRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZRGEZTCMBWGM . You are receiving this because you were mentioned.Message ID: @.***>

coreyostrove commented 11 months ago

Hey @kmrudin and @jordanh6, @rileyjmurray and I did some user-exposure tagging (see above on what that means) of the various algorithms submodules. This included our best guesses for submodule -level visibility for some code that you both own (RB-stuff for @jordanh6 and RPE-stuff for @kmrudin). Could you take a look at the changes in this commit 6565ad70bf846abc6f23d87783ec938c84b089e2 and if you disagree with our assessment let us know?

jordanh6 commented 11 months ago

The RB parts look reasonable to me.