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

optimize split_matrix function #12552

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ivanz851
Copy link

I optimized split_matrix function by removing duplicate code to the extract_submatrix function, and added tests to test_strassen_matrix_multiplication.py file

@algorithms-keeper algorithms-keeper bot added the tests are failing Do not merge until tests pass label Jan 31, 2025
@algorithms-keeper algorithms-keeper bot added awaiting reviews This PR is ready to be reviewed and removed tests are failing Do not merge until tests pass labels Jan 31, 2025
@josmithua
Copy link

This increases readability and maintainability of the code, but certainly does not "optimize" the algorithm, for you have added the extra overhead of calling a function, which will actually slow down the algorithm.

@ivanz851
Copy link
Author

ivanz851 commented Feb 3, 2025

This increases readability and maintainability of the code, but certainly does not "optimize" the algorithm, for you have added the extra overhead of calling a function, which will actually slow down the algorithm.

Actually, my code with extract_submatrix function is not less efficient than the original one, since there is exactly same number of conditional jumps. This can be checked using godbolt.org: comparison. If you follow the link, you can see disassembly of my revision of the code on the left and of the original code on the right. In original code, there are 8 FOR_ITER jumps and 8 JUMP_BACKWARD jumps. In new code, there are 2 FOR_ITER jumps and 2 JUMP_BACKWARD jumps in extract_submatrix function, which is called 4 times, so in sum we have same 8 FOR_ITER jumps and 8 JUMP_BACKWARD jumps. That's why I think code with extract_submatrix function is not slower and can be used for better readability.

@ivanz851
Copy link
Author

ivanz851 commented Feb 3, 2025

@josmithua

@josmithua
Copy link

@ivanz851 Thanks, I learned something new today. Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting reviews This PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants