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

Closed
wants to merge 9 commits into from

Conversation

samuelstjean
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants