Reverse-dependency checks

[thierry-martinez]

We should continue ensuring that the PRs we merge into Graphix do not break the repositories we care about, such as graphix-ibmq, graphix-symbolic, and others. This process is what I refer to as a "reverse-dependency check": verifying that downstream packages depending on Graphix remain functional after any changes we introduce. If something breaks, we should be notified as early as possible.

Currently, we already perform such a check for graphix-symbolic. The noxfile.py configures Graphix's CI to run the graphix-symbolic test suite, ensuring compatibility with every PR.

However, there was no such reverse-dependency check for graphix-ibmq, and as a result, its device interface was broken for some time. This happened because several layers of changes were introduced without ever verifying that the device interface still worked. That is understandable, given that CI wasn't tracking it and nobody manually checked. PR #19 fixes graphix-ibmq to work with the current version of Graphix, but without reverse-dependency checks, similar breakages can occur again in the future.

I believe we should implement reverse-dependency checks not just for graphix-symbolic, but also for graphix-ibmq and any other repositories we care about.

These checks can be implemented in different ways:

• Option 1: Add the dependent repositories' test suites directly into Graphix’s CI (as is currently done for graphix-symbolic), typically by updating noxfile.py.
• Option 2: Trigger the CI of dependent repositories via GitHub workflow dispatch, as @EarlMilktea suggested.

One benefit of the second approach is that the CI workload for these tests is offloaded to the other repositories, rather than being run in Graphix’s CI. However, since the downstream repositories are ours, the cost is effectively shared regardless. In practice, we’ll still need to wait for those CIs to succeed before merging a PR into Graphix.

That said, the second approach introduces more complexity. While triggering downstream CI is relatively straightforward, getting their results back into the Graphix PR discussion requires CI "plumbing" on both ends. By contrast, the first approach is technically simpler and doesn't require changes to the CI of other repositories. Importantly, the feedback from reverse-dependency checks must be visible in the PR discussion thread to ensure it’s considered during review.

[EarlMilktea]

Trigger the CI of dependent repositories via GitHub workflow dispatch, as @EarlMilktea #261 (comment).

I come to feel this approach might be overkill.

broken for some time

I believe we should sometimes allow this for faster v0 development. How about kicking the combined CIs only on tag push or release?

[thierry-martinez]

I believe we should sometimes allow this for faster v0 development. How about kicking the combined CIs only on tag push or release?

I think being notified as soon as possible is preferable:

• if a change in Graphix implies a change in a downstream package, then we can temporarily target a specific branch of that downstream package in Graphix CI,

• if such a change requires a long-term update in the downstream package that is outside the scope of the PR, then we can temporarily skip the test in the CI.

In any case, that forces us to keep track of what is broken and what remains to fix.

Even if the proper fix is postponed for another issue or PR, it is important to associate the breaking with the PR that introduces it, because it belongs to the review process and the authors of the break may necessary be available once their PR is merged (for instance, if they are external contributors).

[EarlMilktea]

I think being notified as soon as possible is preferable:

This is true.

postponed for another issue

Then do you mean, in some situations, PR could be merged even though some reverse CIs are failing?
I'm not against your idea if so.

[thierry-martinez]

Then do you mean, in some situations, PR could be merged even though some reverse CIs are failing? I'm not against your idea if so.

No, if some reverse CIs fail and we want to postpone the fix, then we have to mark the test as skipped, just as we do for any other tests in the test-suite. This way, we keep track of the failure.

[EarlMilktea]

mark the test as skipped, just as we do for any other tests

This is not the same as we cannot do it inside the same PR/repo. I believe ignoring failing CIs is sometimes justified, for instance, when maintainers of downstream packages are so busy that the original PR is expected to be long blocked.

[thierry-martinez]

This is not the same as we cannot do it inside the same PR/repo.

My apologies, I wrote "mark the test as skipped" by analogy with other tests in the test suite, but my idea is to mark the test as deselected, still within the Graphix repo, by modifying the noxfile.py, so that skipped and deselected tests are tracked in the same repository. For instance:

 @nox.session(python=["3.9", "3.10", "3.11", "3.12"])
 def tests_symbolic(session: Session) -> None:
     """Run the test suite of graphix-symbolic."""
     session.install("-e", ".[dev]")
     # If you need a specific branch:
     # session.run("git", "clone", "-b", "branch-name", "https://github.com/TeamGraphix/graphix-symbolic")
     session.run("git", "clone", "https://github.com/TeamGraphix/graphix-symbolic")
     session.cd("graphix-symbolic")
-     session.run("pytest")
+     session.run("pytest --deselect tests/test_sympy_parameter.py::test_parameter_pattern_simulation")

[EarlMilktea]

Sounds good. Thanks!