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 nlmeans indexing #1258

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@samuelstjean
Contributor

samuelstjean commented Jun 16, 2017

I though I had made an issue about it, must have been a mental note.

Anyway, the cython part of nlmeans had a few indexing error which inverted along the way the first and second dimension, so hopefully I fixed them here. That's why not all the code is changed, the main loop was using j,i,k, but passing i,j,k down, so now they should match.

I also revamped the calling file to remove parts I personally deemed unnecessary at the same time. From memory, the example also had some statements which were not truc, so I'll have a look afterwards if this here is fine.

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented Jun 16, 2017

There it is after all #1178. It fixes half the stuff from the issue (as per title), the other half seems to be fixed around in #1245

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented Jun 17, 2017

erm are they fixing it in the other pr? it seemed like nobody understand what I meant, half the indexing in the other loop works, and the one in the inner loop is different. It only has a small effect since it swaps local coordinate, but it is still wrong, that's why you only need to change half of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment