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 for surface rendering function #1604

Merged
merged 3 commits into from
Sep 29, 2016

Conversation

9prady9
Copy link
Member

@9prady9 9prady9 commented Sep 21, 2016

Fixes #1603

@9prady9 9prady9 added the fix label Sep 21, 2016
@9prady9 9prady9 added this to the v3.4.1 milestone Sep 21, 2016
@9prady9
Copy link
Member Author

9prady9 commented Sep 22, 2016

@shehzan10 Apparently, regular modDim backend function is also doing evaluate before it is modifying the meta data of Array.

The problem lies in dims being modified before data is evaluated. Since each JIT node is storing the original dims, when eval is called after moddims is done, the node is still using original dims to compute the array values.

@pavanky Does changing m_dims member variable in buffer JIT node whenever moddims is called have any side effects ?

@pavanky
Copy link
Member

pavanky commented Sep 22, 2016

@shehzan10 didn't we fix this ?

@pavanky
Copy link
Member

pavanky commented Sep 22, 2016

Ok, it looks like me and shehzan found the exact same bug for when inputs are NOT vectors and fixed that. We forgot to fix cases where input is a vector. It is a simple enough fix.

@9prady9 can you look at the following commit and send in a proper patch.
18a4e55

@9prady9
Copy link
Member Author

9prady9 commented Sep 22, 2016

@pavanky The fix is using function version of modDims instead of Array::modDims ?

@pavanky
Copy link
Member

pavanky commented Sep 22, 2016

@9prady9 yes, the function version of modDims is updating the metadata properly. So it is easier to just call that than re-implement changing the metadata in surface.

Use function version of modDims function to properly
update metadata before JIT evaluation is carried out.
@9prady9 9prady9 changed the title WIP - fix for surface rendering function fix for surface rendering function Sep 22, 2016
@9prady9
Copy link
Member Author

9prady9 commented Sep 22, 2016

@shehzan10 This is ready for merge.

@pavanky
Copy link
Member

pavanky commented Sep 22, 2016

@9prady9 can you look to see if any other functions in src/api/c/ are using arr.modDims and change those as well ? (except modDims function ofcourse :) )

@9prady9
Copy link
Member Author

9prady9 commented Sep 22, 2016

Sure

@9prady9
Copy link
Member Author

9prady9 commented Sep 22, 2016

There is only file histeq.cpp that used that and i changed that now.

@shehzan10 shehzan10 merged commit 147a83b into arrayfire:hotfix-3.4.1 Sep 29, 2016
@9prady9 9prady9 deleted the surface_fixes branch September 29, 2016 14:41
@shehzan10 shehzan10 mentioned this pull request Oct 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants