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 segfaulting issue with reduce iterator #382

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

Conversation

lusewell
Copy link

@lusewell lusewell commented Jun 2, 2021

Edit is much smaller than diff suggests - mostly just whitespace due to an extra layer of indentation.

The test added generates the segfault described in #381 without the rest of the change, and then the other change fixes this.

@lusewell
Copy link
Author

lusewell commented Jun 2, 2021

This condition only actualyl needs to be that the last non-zero-strided axis is also the smallest non-zero-strided axis, but this was simpler to write.

@rdbisme rdbisme force-pushed the bugfix/fix-segfault-on-unordered-strides branch from 4e0d60b to 3ab9eb8 Compare February 22, 2022 22:40
@rdbisme
Copy link
Collaborator

rdbisme commented Feb 22, 2022

Hello I rebased this to try to merge it but the test segfaults in the CI...

@lusewell
Copy link
Author

lusewell commented Mar 14, 2022

Removing the change to iterators.h, the tests still cause a segfault - ie current state on master is broken.

The change of reverting the previous release means its quite a lot of effort to work out what the correct fix would now be.

@lusewell
Copy link
Author

EG you've reverted a load of bugfixes such as a8dbfdc, and removed the tests that got added for them...

Why? Can you readd all of these please?

@rdbisme
Copy link
Collaborator

rdbisme commented Mar 14, 2022

Hello, the reason is explained here #388 (comment) . We needed to release some fixes and the state of the main branch was inconsistent.

The previous state is in stored in develop. Feel free to cherry pick the commit you need to fix this.

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