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

Fix CubeMove for rotated cubes #5

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

vvolhejn
Copy link

@vvolhejn vvolhejn commented Oct 5, 2021

Resolves #4. Instead of using get_rounded_center(), now gets the relevant slice of cubies and rotates it using np.rot90. This avoids precision altogether and is also computationally efficient. I tested this by running the "all together" example that takes a cube and solves it using Kociemba's algorithm; the rotations work well even if the cube is not axis-aligned. I did not observe any numerical errors accumulating.

In addition:

By the way, I think the now-removed lines

    cubies = np.ndarray
    indices = {}

in RubiksCube do something else than what was intended - it's not default values that get initialized for each instance, but rather class attributes that are shared among instances. So this would probably cause trouble if we tried to animate multiple cubes. Also, cubies = np.ndarray assigns the type np.ndarray to the variable, which seems unintended.

@WampyCakes
Copy link
Owner

Rather than changing the axis of rotation, my eventual plan was to change from indexing based on rounded locations to using some index independent of world space. It's been a while since I've thought about it, so I can't remember my exact idea. Perhaps it was similar to what you proposed here:

A more robust fix would be to find the nearest neighbor of each of the new cube positions and match the old and new positions this way.

Would you like to try implementing this? If so, the goal of all code in this plugin (and yes, I know this hasn't been accomplished in practice as much as I'd like) is to be computationally efficient so that very large n-dimensional cubes aren't overly slow to work with.

@vvolhejn
Copy link
Author

I'm beginning work on a Rubik's cube manim video, so I need to implement this one way or another - I'd be happy to help with this. I also suspect that my fix proposed here still causes problems.

The computational bottleneck will almost certainly be rendering rather than recomputing the indices, unless we're talking 500x500 cubes or something of that sort. So I wouldn't worry about that too much.

I'll update the code soon.

@vvolhejn vvolhejn changed the title Fix CubeMove for rotated cubes WIP: Fix CubeMove for rotated cubes Oct 18, 2021
@WampyCakes
Copy link
Owner

What's the video going to be about?

I agree that rendering is the bigger problem, but as rendering speed improves with OpenGL, I'd prefer to keep the computational side as light as possible as well.

@vvolhejn
Copy link
Author

The video is going to be about how to algorithmically solve the cube in the shortest number of moves using a computer program. I'll let you know when it's done!

Do you have any plans to implement OpenGL rendering?

I've fixed the rotation now using a method that doesn't use rounded coordinates, relying instead on np.rot90. See the updated pull request description for the full changes.

@vvolhejn vvolhejn changed the title WIP: Fix CubeMove for rotated cubes Fix CubeMove for rotated cubes Nov 5, 2021
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.

CubeMove breaks when the cube is rotated
2 participants