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

Remove RotatedHyperrectangle #3592

Merged
merged 1 commit into from
Sep 5, 2024
Merged

Conversation

schillic
Copy link
Member

This set type has never been used, so I suggest to remove it.
(We can always recover the files from the history if desired at a later point.)

@schillic schillic marked this pull request as ready for review July 17, 2024 19:56
@schillic schillic force-pushed the schillic/RotatedHyperrectangle branch from 6077048 to a579445 Compare August 3, 2024 04:59
@schillic schillic force-pushed the schillic/RotatedHyperrectangle branch from a579445 to 8936f8a Compare August 24, 2024 06:12
@mforets
Copy link
Member

mforets commented Aug 24, 2024

Type that represents a hyperrectangle that is not necessarily axis aligned.

This is a special kind of zonotope, I wonder if there are some specific features or reasons why it was added (i dont remember).

@schillic
Copy link
Member Author

I added it back then and I think the motivation was a reachability algorithm for discrete systems where you start from a hyperrectangle and only apply linear_maps. But this is so specific that it is probably not worth keeping this type for it, even in this special case the savings are probably not significant, and we never actually used it. Here are the main differences:

  • The Zonotope type implicitly uses the unit hypercube. Instead, a RotatedHyperrectangle uses an arbitrary hyperrectangle, but stores it explicitly. I think in most cases this additional storage is just overhead, but starting from a hyperrectangle is easier.
  • The name RotatedHyperrectangle is very misleading. In fact, it can take the shape of any Zonotope because the matrix can be rectangular.
  • Additionally, the Zonotope stores a translation vector, which the RotatedHyperrectangle does not. So for every linear_map operation you save a matrix-vector multiplication, but the translation operation is difficult instead (we do not even have it implemented).

In summary, I do not oppose keeping the RotatedHyperrectangle type, but since I was the one adding it and never used it, I am open to removing it. Reducing the code base has benefits as well.

@schillic schillic added the breaking ❌ This change may break things label Aug 27, 2024
@schillic schillic merged commit 853a9aa into master Sep 5, 2024
5 of 6 checks passed
@schillic schillic deleted the schillic/RotatedHyperrectangle branch September 5, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking ❌ This change may break things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants