Type mismatch for `tgt_coords` between `check_skip_move` and `check_critical_beams`

[xbb]

The tgt_coords parameter in the check_skip_move function is annotated as list[StandardCoord] but check_critical_beams expects a single StandardCoord for the same parameter.

Inside check_critical_beams, nxt_coords is compared with tgt_coords, which will always evaluate to False because they are different types, list vs single item.

[jbolns]

Great catch. It follows that the entire block under that conditional is not really doing much, or anything at all for that matter.

In turn, there is a need to check the runtime/completion trade-offs of either correcting the mismatch so the block actually kicks-in or entirely getting rid of it.

@xbb , Would you happen to have a computer with good processor and some RAM?

With a good processor and 16/32 RAM, one can realistically run tests using docs/examples/pyzx_random.py with up to depth=50. Else, it does become somewhat unfeasible given runtimes (we working on that, and this block is relevant for what we're discussing).

[xbb]

Hello, on a mini PC with an Intel i5-1240P CPU, running in a Debian VM, the pyzx_random test with seed 42 and depth 50 settings takes about ~1020s.
With the fixes applied there is no meaningful difference. (I'm passing tgt_coords[0] to check_critical_beams if it makes sense + fix for #38 )

On a Mac mini M2 it completes after ~1180s.

[jbolns]

Cool. I checked the PR quickly on mobile.

• Like the sys logging. Topologiq needs a lot more of that to later wrap with some proper logging.
• Also good to send the single coordinate rather than changing typing in function.
• But, if no significant difference is seen, best to get rid of the block entirely and adjust function parameters as applicable (can't recall by memory which parameters will become unnecessary).

[jbolns]

Ha. I just realised it was a referenced commit and not a PR. Note to self. Do not check GitHub on mobile. 🤣

Anyway, comments apply.

Also, if you feel confident in your GitHub skills, could you add the changes on top of branch refactor_36 either by (preferably) pulling it and applying changes there to then push back, or (carefully) merging into that branch instead of main?

That branch is ready to merge, so won't see any more changes.

However, there's some overlaps, so this would make it easier to avoid potential conflicts once both streams of work reach main.

[jbolns]

Thanks @xbb . The related PR that you submitted was merged alongside #39 !
While it's an educated guess, I liked that the bugs you found suggest you're engaging with one of the most complex abstractions that make or break Topologiq. I look forward to your next suggestions.