fastrow and fastcolumns documention and the defaults for memory layout stated clearly#635
fastrow and fastcolumns documention and the defaults for memory layout stated clearly#635arnavk23 wants to merge 31 commits intoQuantumSavory:masterfrom
fastrow and fastcolumns documention and the defaults for memory layout stated clearly#635Conversation
Co-authored-by: Stefan Krastanov <github.acc@krastanov.org>
arnavk23
changed the title
fastrow fastcolumn docsfastrow and fastcolumns documention and the defaults for memory layout stated clearly
|
@Hamiltonian-Action can you review this pr too. Thanks! |
|
Probably better to leave this in the hands of Stefan as he'd be more familiar with what the desired outcome would be but I will leave a few remarks. |
docs/src/datastructures.md
Outdated
|
|
||
| The alternative **column-major (fastcolumn) layout** stores tableau columns (mostly) contiguously in memory. This layout is optimized for: | ||
| - **Applying sparse gates** like `apply!(s, sCNOT(i,j))` - row updates on a few qubits | ||
| - **Pauli multiplications** (left or right) |
There was a problem hiding this comment.
Do we actually have the data for this? From the GPU side, I would reckon that both styles should execute nearly equally as fast if implemented properly (assuming no phase information is desired, otherwise row-major would likely reign supreme)
Moreover, row-major multiplication is more straightforward to parallelise/vectorise/distribute/etc. as it eliminates any inter-task dependency.
There was a problem hiding this comment.
I think these are all the benchmarks that exist (and they are pretty outdated and not auto-executed in CI):
Thanks, @arnavk23 , the enthusiasm you are showing in these PR is very heartening! Apologies for not having had the time to review more of your work (and thank you, @Hamiltonian-Action for picking up the slack, that is much appreciated!) @arnavk23 , I will try to get to the actual review of this and the other PR in the next couple of days. To address your more general questions: Your profile would lead to a strong gsoc application. In terms of the review process (the main way in which others can evaluate your skills) I have two main suggestions: (1) Answer comments a bit more explicitly instead of just making changes to the code. E.g. each comment in a PR review starts its own thread. Unless the comments are completely trivial, answer them with your thoughts so that the reviewer can get a better idea of whether you understand the issue. (2) Minimize the use of LLMs until you are familiar with a code base. Otherwise it is difficult for the reviewer to trust that you have vetted the code from the LLM. And it is difficult to know whether you are engaging with the discussion or just sending the review comments straight to the LLM without understanding them. Do not get me wrong, these tools are great and I use them too. But during review I need to trust you that you are using such tools judiciously. If I see a silly mistake I do not know whether you made it and will learn from it (in which case the silly mistake is not a problem at all) or whether the LLM made the mistake and I am the only one that is looking out for such mistakes (which is much more worrisome, because LLMs do not learn from their mistakes). Thanks again for the submissions. I will try to review and merge in the next couple of days. |
Thank you @Krastanov for your reply, I will surely keep these suggestions in mind working further on this repo and any other repo I ever work on. I am really enjoying going through the work done here by you and your lab and would love to contribute more. Thank you for everything. |
Krastanov
left a comment
Krastanov
left a comment
There was a problem hiding this comment.
Thanks, this is on the way of becoming a really nice informative page!
You picked a good set of low-level functions to gain understanding of the codebase -- reinterpret/fastrow/fastcolumn can be subtle.
I left a few comments in. Please make sure to address HA's comments as well.
Feel free to convert to a non-draft when it is ready for review.
|
This related PR would also be a good example of some such tests https://github.com/QuantumSavory/QuantumClifford.jl/pull/478/changes |
- Clarified memory layout concepts in user documentation - Removed vague implementation details and outdated benchmark references - Added concrete @time examples showing canonicalization performance differences - Removed unfounded GPU performance claims without supporting data - Renamed test to 'memory layout correctness' to reflect its actual purpose - Documented that row-major layout shows measurable advantage on CPU for large states - Added guidance to use profiling for layout-specific optimization decisions
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #635 +/- ##
==========================================
+ Coverage 73.14% 74.05% +0.91%
==========================================
Files 108 108
Lines 7335 7224 -111
==========================================
- Hits 5365 5350 -15
+ Misses 1970 1874 -96 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Explicitly document that fastrow is the default for Stabilizer, Destabilizer, MixedStabilizer, and MixedDestabilizer - Explicitly document that fastcolumn is the default for PauliFrame - Explain technical implementation: fastrow uses column-major storage, fastcolumn uses row-major storage - Add indexing convention that applies regardless of layout: xzs[tableau_column_index, tableau_row_index] - List which operations are optimal for each layout - Update docs to use 'fastrow vs fastcolumn' terminology instead of 'row-major vs column-major' - Add technical details section explaining how storage order changes between layouts Addresses QuantumSavory#635
3 participants
If you want to submit an unfinished piece of work in order to get comments and discuss, please mark the pull request as a draft and ping the repository maintainer.
Please address only one topic or issue per pull request! Many small PRs are much easier to review and merge than one large PR.
Before merging, all changes and new functionality should be marked in the CHANGELOG file, but feel free to just leave your CHANGELOG notes in the PR description, to avoid merge conflicts with other requests modifying that file. The maintainer will add these CHANGELOG notes for you if you do so.
Before considering your pull request ready for review and merging make sure that all of the following are completed (please keep the clecklist as part of your PR):
If possible, keep your git history not too wild (rebase and squash commits, keep commits small and semantically separated) so that review is easier.