Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New AffineShiftOp Modelmember and Re-implementation of ProtectedArray #386

Merged
merged 6 commits into from
Jan 31, 2024

Conversation

coreyostrove
Copy link
Contributor

@coreyostrove coreyostrove commented Jan 8, 2024

This PR introduces a new operation modelmember called AffineShiftOp. As the name implies, this operation is one that induces an affine shift when applied to a vector. The general structure of these operations is all ones along the diagonal and non-zero entries in the first column. This operation is useful in the context of constructing FOGI representations for SPAM. The only free/mutable parameters of this operation are the non-diagonal elements of the first column of the process matrix.

The implementation of AffineShiftOp is similar to that of FullTPOp in that it uses ProtectedArray to impose read-only access on elements whose values are fixed according to the general structure of the operation class. In this case that restriction is on all elements other than the non-diagonal entries of the first column. The original implementation of ProtectedArray wasn't able to handle this case, however, as it required that the protected indices correspond to a sliceable sub-block of the original array. Generalizing the original implementation in order to handle more general protected index structures proved quite challenging, so in the end I decided to re-implement much of ProtectedArray from scratch to enable this support with much less difficulty. The new version uses a mask-based implementation to allow arbitrary index protections to be set. For what it's worth, I think the new implementation is also much easier to understand and simpler overall. That said, messing with something in baseobjs should be done with care, so having some extra attention on these changes would be appreciated.

Finally, I've added in some new unit tests for AffineShiftOp and for the new functionality in ProtectedArray.

Update 1/31/24: I've manually run the test_packages workflow and can confirm that these tests are all presently passing as well.

Corey Ostrove added 3 commits January 5, 2024 22:55
This commit adds in a new modelmember called AffineShiftOp for modeling operations that correspond to purely affine shifts of a vector in Hilbert-Schmidt space. This is useful as a building block for the new ASMP representation being used in certain FOGI contexts. As part of this update the implementation of ProtectedArray has been significantly rewritten to allow a more general specification of protected indices
which is needed for restricting to the form of the affine matrices in question.
Fix some edge cases in the protected index mask construction logic identified during unit testing.
Add in new unit tests for AffineShiftOp and a new unit test for ProtectedArray's new mask-based implementation.
@coreyostrove coreyostrove added this to the 0.9.13 milestone Jan 8, 2024
@coreyostrove coreyostrove self-assigned this Jan 8, 2024
@coreyostrove coreyostrove marked this pull request as ready for review January 31, 2024 18:18
Copy link
Contributor

@sserita sserita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and with test updates too! @coreyostrove awesome work :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner!

@sserita sserita merged commit 932118f into develop Jan 31, 2024
16 checks passed
@sserita sserita deleted the feature-affine-shift-op branch January 31, 2024 18:29
@sserita sserita modified the milestones: 0.9.13, 0.9.12.1 Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants