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

Unexpected Output from delete from array #700

Merged
merged 3 commits into from
Jun 16, 2022

Conversation

amehra-ni
Copy link
Contributor

@amehra-ni amehra-ni commented Jun 13, 2022

Unexpected Output from delete from array BUG
Delete from array doesn't work correctly for non-flat arrays(>=2-d array) if in a loop. I have attached the behavior below.

dfa

cv.mp4

Delete from array works fine for the first iteration, but after the first iteration, the values in the input array somehow get deleted which also makes the deleted and output array reflect the same. In the second iteration when ResizeDimension is called on deletedArray(line 1316) and arrayOut(line 1323), and getInit happens for them, it basically initializes the input array's values as well which should not have happened. We found out this happens because deletedArray and arrayOut don't get copied correctly(from the copy subarray function) and the values are still related to the input array. So when getInit happens for the deleted and output array and their values are getting initialized and as they still point to the input array, values of the input array get changed.

In this PR, we have fixed the issue by changing how the non-flat array gets copied from the CopySubarray function. The changes are done for non-flat type arrays and for flat type it's still the same. I have used what we have for elementType->CopyData so that a proper copy(deep copy) happens.

I have tested the changes manually with different cases and also have added a test. The reason I didn't add it in ArrayDelete.via was as I have added a loop as well just to be sure. Attached the via below
3

CI Build in my branch - build (Lint is failing because of 4 dead links. I tried searching why is it happening but didn't find anything significant)
Run the squad tests - pipeline
@shivaprasad-basavaraj

@amehra-ni amehra-ni changed the title Users/amehra/array delete fix copy subarray function update Jun 13, 2022
@amehra-ni amehra-ni changed the title copy subarray function update Unexpected Output from delete from array Jun 13, 2022
@sanmut
Copy link
Contributor

sanmut commented Jun 14, 2022

Please do add more VI based tests in the test suite in its repo and validate. Also, it is a good idea to test those VIs against NXG Desktop or CurrentGen.

@amehra-ni
Copy link
Contributor Author

Sure @sanmut, I'll add tests in NXG codebase.

Copy link
Contributor

@cglzaguilar cglzaguilar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll defer to Spathiwa and Samu on the changes. Please address their feedback.

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.

5 participants