search

sandialabs / pyGSTi

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

test_fiducialpairreduction takes a very long time #360

Closed rileyjmurray closed 7 months ago

rileyjmurray commented 8 months ago

The tests defined at https://github.com/pyGSTio/pyGSTi/blob/d158976317310156316db8922804bbc2a4dbbf73/test/unit/algorithms/test_fiducialpairreduction.py take several minutes to run on my Macbook Pro with an M2 Max CPU. This makes it impractical to casually run pytest test/unit/algorithms and see if some small changes to code broke anything or fix anything.

I'm filing this issue so that I (or something else) remembers to make this test more efficient at some point.

coreyostrove commented 8 months ago

Thanks for raising this issue, @rileyjmurray. I'm familiar with the relevant knobs here and should be able to get the runtime trimmed down. I clocked this set of tests at 9:33 on a single thread on my x86-based machine. Some low-hanging fruit I noticed right away:

  1. FPR typically doesn't do anything to the idle gate, so testing FPR with a gate set that includes the idle is a waste of computation.
  2. For the randome FPR routines we can significantly reduce the number of per-iteration samples taken, as the resulting reduced sets don't actually need to be good for the sake of testing.

I'll make those changes and report back on the updated testing times (we can decide if we need any more pruning at that point).

coreyostrove commented 8 months ago

Some updates:

All in all the algorithms module now takes ~1:45 on a single thread on my laptop. Current time outliers are the gauge optimization tests in test_gaugeopt.py. So I'll take a quick look at those before pushing up a branch with these changes.

rileyjmurray commented 8 months ago

That sounds great! Getting tests down below 2 minutes is very reasonable.

On Thu, Nov 9, 2023 at 6:15 PM coreyostrove @.***> wrote:

Some updates:

  • I've streamlined the tests in test_fiducialpairreduction.py and have got the runtime for that set of tests down to ~15 seconds on a single thread on my machine (down from 9:33 originally).
  • As part of those updates I did some refactoring of the test fixtures to use a non-legacy modelpack. That broke a bunch of stuff so I had to refactor a good chunk of the remainder of the algorithms test module and implement some bugfixes in the actual codebase to fix that.
  • I also noticed some coverage lapses in test_fiducialselection, which plugged a bit.

All in all the algorithms module now takes ~1:45 on a single thread on my laptop. Current time outliers are the gauge optimization tests in test_gaugeopt.py. So I'll take a quick look at those before pushing up a branch with these changes.

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

rileyjmurray commented 8 months ago

@coreyostrove can you push these changes to a branch so I can pull them down?

coreyostrove commented 8 months ago

These changes are live on the branch 'feature-faster-algorithm-tests' (same branch I've been doing my debugging for #369, fwiw). Sorry for not following up to indicate that I had pushed this.

P.S. Because I am doing the debugging on #369 the yamls for the CI workflow on github runners are currently tweaked on this branch, so be mindful of that to avoid accidentally propagating those changes to other branches.

sserita commented 7 months ago

Closed with release of 0.9.12.