{"action":"created","issue":{"url":"https://api.github.com/repos/DynamoDS/DynamoRevit/issues/758","labels_url":"https://api.github.com/repos/DynamoDS/DynamoRevit/issues/758/labels{/name}","comments_url":"https://api.github.com/repos/DynamoDS/DynamoRevit/issues/758/comments","events_url":"https://api.github.com/repos/DynamoDS/DynamoRevit/issues/758/events","html_url":"https://github.com/DynamoDS/DynamoRevit/pull/758","id":114764735,"number":758,"title":"[DNM]Implement MAGN-3427 Update Adaptive Component Placement to use new Batch creation method","user":{"login":"Randy-Ma","id":5584246,"avatar_url":"https://avatars.githubusercontent.com/u/5584246?v=3","gravatar_id":"","url":"https://api.github.com/users/Randy-Ma","html_url":"https://github.com/Randy-Ma","followers_url":"https://api.github.com/users/Randy-Ma/followers","following_url":"https://api.github.com/users/Randy-Ma/following{/other_user}","gists_url":"https://api.github.com/users/Randy-Ma/gists{/gist_id}","starred_url":"https://api.github.com/users/Randy-Ma/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Randy-Ma/subscriptions","organizations_url":"https://api.github.com/users/Randy-Ma/orgs","repos_url":"https://api.github.com/users/Randy-Ma/repos","events_url":"https://api.github.com/users/Randy-Ma/events{/privacy}","received_events_url":"https://api.github.com/users/Randy-Ma/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-11-03T07:59:04Z","updated_at":"2015-11-04T13:30:37Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/DynamoDS/DynamoRevit/pulls/758","html_url":"https://github.com/DynamoDS/DynamoRevit/pull/758","diff_url":"https://github.com/DynamoDS/DynamoRevit/pull/758.diff","patch_url":"https://github.com/DynamoDS/DynamoRevit/pull/758.patch"},"body":"### Purpose\r\n\r\nThe purpose of this pull request is to implement three new nodes for creating adaptive components in a batch. To create a number of adaptive components, originally they have to be created one by one by various lacing strategies. Now NewFamilyInstanceCreationData can be used for adaptive components. A number of creation data can be prepared first and the adaptive components can be created in batch by NewFamilyInstances2.\r\n\r\nThree new nodes are introduced. These nodes are added based on the existing three nodes. The difference is that now the new nodes are taking two-dimensional array as the first parameter. The first dimension identifies how many adaptive components are to be created. The second dimesnion identifies how many adaptive points are for each component. For example, a[100][4] means to create 100 adaptive components and each of them has 4 placement points. Internally the node is using this two-dimensional array and create a number of adaptive elements in batch. @mccrone Do you have any suggestions for the node names or the input parameters?\r\n![image](https://cloud.githubusercontent.com/assets/5584246/10902989/9d6c832e-823f-11e5-9222-a4cbb968a92f.png)\r\n\r\nFor example to place the following adaptive component around a semi-sphere: (This is a pretty simple adaptive component family which is composed of four bars and has 4 adaptive points)\r\n![image](https://cloud.githubusercontent.com/assets/5584246/10903683/36290630-8247-11e5-8a68-383b518607a8.png)\r\n\r\nThe used node is BatchByParametersOnFace. A semi-sphere surface is divided into a number of parts according to u and v equally. Each part has four points. These four points will be used to place an adaptive component. The result is:\r\n![image](https://cloud.githubusercontent.com/assets/5584246/10903655/ed31aff4-8246-11e5-8e47-80280f2fc962.png)\r\n\r\n##### Performance Improvement\r\n\r\nThe performance improvement is not apparent if only a few number of adaptive components are created. But in a case where a large number of adaptive components are created, the observed time taken is 18 min and 10 min for the old nodes and new nodes. The new nodes do have performance advantage over the old ones.\r\n\r\n##### Known Limitation\r\nRight now every time the new nodes are run, all old elements created by the nodes will be deleted. This is because if we try to re-use the old elements by updating them, then we need to update them one by one. This will take away the benefit we gained by processing them in batch.\r\n\r\n\r\n### Declarations\r\n\r\nCheck these if you believe they are true\r\n\r\n- [X] The code base is in a better state after this PR\r\n- [X] Is documented according to the [standards](https://github.com/DynamoDS/Dynamo/wiki/Coding-Standards)\r\n- [X] The level of testing this PR includes is appropriate\r\n- [ ] User facing strings, if any, are extracted into `*.resx` files\r\n- [ ] All tests pass using the self-service CI.\r\n\r\n### Reviewers\r\n\r\n@sharadkjaiswal \r\n@mjkkirschner \r\n\r\n### FYIs\r\n\r\n@kronz @mccrone @ikeough "},"comment":{"url":"https://api.github.com/repos/DynamoDS/DynamoRevit/issues/comments/153719609","html_url":"https://github.com/DynamoDS/DynamoRevit/pull/758#issuecomment-153719609","issue_url":"https://api.github.com/repos/DynamoDS/DynamoRevit/issues/758","id":153719609,"user":{"login":"kronz","id":3456818,"avatar_url":"https://avatars.githubusercontent.com/u/3456818?v=3","gravatar_id":"","url":"https://api.github.com/users/kronz","html_url":"https://github.com/kronz","followers_url":"https://api.github.com/users/kronz/followers","following_url":"https://api.github.com/users/kronz/following{/other_user}","gists_url":"https://api.github.com/users/kronz/gists{/gist_id}","starred_url":"https://api.github.com/users/kronz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kronz/subscriptions","organizations_url":"https://api.github.com/users/kronz/orgs","repos_url":"https://api.github.com/users/kronz/repos","events_url":"https://api.github.com/users/kronz/events{/privacy}","received_events_url":"https://api.github.com/users/kronz/received_events","type":"User","site_admin":false},"created_at":"2015-11-04T13:30:37Z","updated_at":"2015-11-04T13:30:37Z","body":"Sorry, the conversationis happening now in 2 places, but, yes @andydandy74  we are pursuing this like you recommend (and to anwer your question about why consider a toggle:  sometimes a batching process can take longer for small numbers of elements or be more complicated to set up than a regular process.  In the end we are not pursuing that avenue anyway).  @Randy-Ma let's keep it here, as getting feeback like this from Andreas is really helpful.  Copy/pasted the side conversation below.\r\n\r\nZhaoyang Ma  04 Nov 2015, 18:12\r\nRight now, the part of work to use only one set of nodes are done:\r\n1). AdaptiveComponent.ByPoints, AdaptiveComponent.ByParametersOnFace and AdaptiveComponents.ByParametersOnCurveReference are modified to take one more dimension for uv, point or parameter. \r\nThe return value is changed to an array.\r\n2). Element binding is implemented to reuse old elements created in the last run as much as possible. But the old elements are updated one by one.\r\n3). There are some invisible overloads which are not visible in the library and I have updated them as well.\r\n(Refer https://github.com/DynamoDS/Dynamo/pull/2414 for details)\r\n4). I discussed with Sharad Jaiswal. He has a good suggestion about migration: for each of the node, add an AllowRankReduction attribute, so that if it returns one element before, now it won't return an array which contains only one element. Also as the engine will do rank promotion, it is very likely that we do not need to spend time on migration.\r\nZhaoyang Ma  04 Nov 2015, 18:18\r\nZach Kron \r\nBut if you really want to add an additional parameter to toggle to use batch method, I will make the change tomorrow.\r\n\r\nZach Kron  04 Nov 2015, 20:18\r\nZhaoyang Ma Nope, that is fine to leave out the additional parameter.  That was only another suggestion to use if there was going to be a limitation around element binding, but with modify using the non-batch method, then we can keep the nodes as they are.  I think at this point the best thing to do is get a build out to me and QA to test it for performance improvements.  The questions I have right now are\r\n- does migration happen seamlessly\r\n- can users still use the same structure for passing in data as before\r\n- with batch creation and regular modification, does modify now take LONGER than creation?  If so, we will need to revisit the implementation.\r\n\r\nThanks, sorry this is turning out to be more complicated than we thought.\r\nZ"}}