Feat/rust compression

[mathleur]

Description

Contributor Declaration

By opening this pull request, I affirm the following:

• All authors agree to the Contributor License Agreement.
• The code follows the project's coding standards.
• I have performed self-review and added comments where needed.
• I have added or updated tests to verify that my changes are effective and functional.
• I have run all existing tests and confirmed they pass.

[peshence]

Nice!

Can you clarify the interplay between the Qube::compute_structural_hash and NodeRef::structural_hash functions? They seem to do very similar things with duplicated code, but also have subtle differences (e.g. version in one but not the other) which I find a bit confusing.

Are you sure we want to hash the leaf coords and the branch child coords, surely those are already in the child hash..? I am not confident here, sorry if it's a nonsensical question

I think if we clean this up it will be easier to review and work with down the road. So e.g. remove extra implementations, commented out code, debug println-s, ideally add tests to CI and some docstrings. Happy to help too if you'd like to delegate something.

[mathleur]

I've just formatted the code and added CI/QA so hopefully it should fix the last comment.

For the compute_structural_hash, I think they're mostly the same, but I forgot some smaller details in the NodeRef one (eg also to sort the children hashes in the NodeRef definition). I'm not using the structural_hash of the NodeRef at all however, and I'm not too sure if we'd ever really use it there? I see the NodeRef more as an external structure for users to access a view of the nodes, and I'm not sure if they'd access the hash as it's quite internal to the Qube operations, but I'm not sure.
I'm happy to harmonise the two definitions now in the meantime 😄

For the structural hash, I think we only want to hash the structure of the node itself, but keeping the coordinates of all of the leaves under it. So in the definition, we do not hash the coords of the node, but we do hash all of the coordinates of the leaves, and so it's important to explicitly add the coordinates to the hash there. We mostly use the hash to identify when nodes can be merged, which is why we ignore the nodes' coordinates.

I think though, we might potentially not need such a deep recursive hash definition, since we know (/assume) that the nodes underneath will be maximally compressed. But I'm not 100% sure.

The recursive hash was a bit difficult to deal with, especially in the compression. This is because we modify the tree in place while recursing from the bottom up, and both the nodes and hashes are constantly changing, so we're trying to access something inside of the recursion that is changing as we recurse. Instead, I had to not remove nodes from the tree while I recurse, and just set the coordinates to be empty for the "dead" nodes, before cleaning up those nodes at the end. The problem however is that even then, we are still not guaranteed that the tree is compressed, because if we had weird duplicate nodes with a different number of children eg

├── class=1
    ├── expver=0001
    │   ├── param=1
    │   └── param=2
    ├── expver=0002
    │   ├── param=1
    │   └── param=2
    └── expver=0002
        ├── param=1
        ├── param=1
        └── param=2

then the hash for the two expver=0002 nodes is not the same, even when we compress in the middle, because they have a different number of children (because we did not remove the children while we compressed). So we have to loop a second time, once we have removed the dead nodes, to remove all duplicates. This is very probably not optimal, so I'm very happy to hear ideas on how to improve 😄