ENZYME-APD / tapir-archicad-automation

The easiest way to use the JSON/Python API from Archicad without knowing how to code.
MIT License
57 stars 17 forks source link

GetPropertyValuesOfElementsComponent #202

Closed tlorantfy closed 2 weeks ago

tlorantfy commented 2 weeks ago

image

tlorantfy commented 2 weeks ago

@kovacsv , Grasshopper freezes when I try to connect AllProperties with GetPropertyValues, see below video. It takes around 1 minute to evaluate :D

https://github.com/user-attachments/assets/12fd9590-7b56-4e05-8e3f-8278ab97930f

kovacsv commented 2 weeks ago

@kovacsv , Grasshopper freezes when I try to connect AllProperties with GetPropertyValues, see below video. It takes around 1 minute to evaluate :D

GetPropertyValues.mp4

Based on the code it shouldn't work at all. PropertyId is a list, and that input accepts an item only.

tlorantfy commented 2 weeks ago

Based on the code it shouldn't work at all. PropertyId is a list, and that input accepts an item only.

Why shouldn't work? Grasshopper works like this: if the input accepts an item but it gets a list then SolveInstance function of the component will be executed N times, for each item in the list. So it works correctly now, but slow for huge number of properties.

I implemented this because the existing SetPropertyValues component accepts a propertyId item also.

But probably it's better to change the input to accept list of propertyIds for GetPropertyValues node. That way the Tapir command will be executed only once when getting values for N properties of M elements. For SetPropertyValues it makes the inputs more complicated...

kovacsv commented 2 weeks ago

Based on the code it shouldn't work at all. PropertyId is a list, and that input accepts an item only.

Why shouldn't work?

Grasshopper works like this: if the input accepts an item but it gets a list then SolveInstance function of the component will be executed N times, for each item in the list.

So it works correctly now, but slow for huge number of properties.

I implemented this because the existing SetPropertyValues component accepts a propertyId item also.

But probably it's better to change the input to accept list of propertyIds for GetPropertyValues node. That way the Tapir command will be executed only once when getting values for N properties of M elements.

For SetPropertyValues it makes the inputs more complicated...

Thanks for the clarification. I thought item input can't accept lists, but it seems like it can. Then it's not a big surprise that it's slow, it runs 1600 separate calls to Archicad.

tlorantfy commented 2 weeks ago

image

Updated, now it executes the GetPropertyValuesOfElements Tapir command only once, even if getting values for multiple properties. It's much more faster now.

Notes: