Fix non-array item search for dot operation

[mmisol]

Purpose

When an array is passed to the dot operation, it needs to get an item
to determine the actual class being processed. This was done in
ArrayUtils.GetFirstNonArrayStackValue, but the function only checked
the first item of the array and its descendants. This caused the dot
operation to failed when passed an array that contained an empty array
as its first item.

In order to fix the problem, the function was changed to check for
non-array items in the entire array. The function should stop as soon
as the first non-array item is found.

Declarations

Check these if you believe they are true

• The codebase is in a better state after this PR
• Is documented according to the standards
• The level of testing this PR includes is appropriate
• User facing strings, if any, are extracted into *.resx files
• All tests pass using the self-service CI.
• Snapshot of UI changes, if any.
• Changes to the API follow Semantic Versioning and are documented in the API Changes document.

Reviewers

@DynamoDS/dynamo

[mjkkirschner]

@mmisol can you design a pathologically bad test case for this change?

[mjkkirschner]

I think we'll need to be careful here regarding performance - I remember that @reddyashish and @aparajit-pratap implemented a few solutions for similar issues that worked but had surprising performance issues with actual user data.

[mmisol]

@mjkkirschner Do you have any suggestions? Just wanted to note that the function is not slower than what it was in scenarios where it actually worked.

[mjkkirschner]

@reddyashish might remember better than me, or @Amoursol may have a better sense for a general limit to nesting in common user workflows.

What happens if the array only contains an empty array 1000 levels deep as the last item? Will it iterate through the entire array?

[mmisol]

What happens if the array only contains an empty array 1000 levels deep as the last item? Will it iterate through the entire array?

It would. Should we not use recursion to contemplate that level of nesting?

[reddyashish]

I am not aware about the general limit on nesting in user workflows but it makes sense for this function to be recursive. We would need the input type for the zeroth nesting level and that would set the replication criteria.

Lets run the performance tests with these changes and see if that shows any difference?

[mmisol]

@mjkkirschner Would you say these changes are good to go? Let me know if not. Thanks!

[QilongTang]

@mmisol Can you talk about the average scale of problem size we are addressing here? I do agree this code is performance sensitive but I also would like to get a sense of how many entries we are iterating usually.

[mmisol]

@QilongTang I think I can give you a theoretical idea of the order of this algorithm. Being n the number of nodes and p the number of levels of the input, you could say best case scenario is o(p) and worst case scenario is o(n).

To give you an average I would have to base on an actual dataset, which I don't think we have. However, worst case scenarios are triggered by extremely sparse inputs, which my intuition tells me might be uncommon. Therefore I estimate the average would continue being o(p)

[mjkkirschner]

let's merge it and pay attention to the performance tests.

[mmisol]

@mjkkirschner Can you please approve?

[QilongTang]

@mmisol Thanks for the explanation, I'm more confident now. LGTM

[Amoursol]

@mjkkirschner @mmisol Did you still need clarity on what kind of nesting users will do with DesignScript? If I'm reading this right that is!

[mmisol]

@Amoursol In the end we settled for running the performance tests after merging this.

I guess the question Mike raised was about how deep was the deepest nested List you have seen in practice in Dynamo. For instance, have you ever seen a List of List of List ... of List with ... representing a repetition of around 1000 levels? I hope not :)

[Amoursol]

The deepest List I've seen in practise is around 4-5 levels - I've seen some random things beyond that on the on the Forum but we probably don't need to worry about it. In reality managing lists that deep is pretty difficult for users.

Typically it's List / List of Lists / List of List of Lists.