Revit_Toolkit: Handling of parameters vs properties on Push/Pull

[pawelbaran]

Detailed issue:

Currently CRUD in Revit_Toolkit works in the following way:

• on Read the Revit element is converted into a BHoM object, which implies loss of some Revit-specific information (e.g. a beam with driving curve aligned to its top face will be pulled as a Physical beam with driving curve aligned to its centroid etc.)
• after Revit element => BHoM object conversion on Read, all Revit parameters are copied over to the CustomData of a BHoM object, resulting in duplication of some information (e.g. pulled Level will have Elevation property and Elevation key in CustomData)
• on Create the Revit element is first created based on a BHoM object and then the CustomData is copied over to its parameters, resulting in a risk of changing the originally created element
• on Update location of the Revit element is first updated based on a BHoM object and then the CustomData is copied over to its parameters, which can easily conflict with each other

To make it more complex, very often a single BHoM property is not 1:1 derived from Revit parameters, e.g.:

• location of a beam in Revit is defined by a combination of its driving curve and multiple parameters (attachment to other elements, offsets, justification etc.), in BHoM it is a single curve
• material of an element is defined by a combination of Material parameter and Material Grade parameter
• in language version other than English, Elevation of a BHoM Level would be 1:1 correspondent with parameter under different name (Rzędna in 🇵🇱)

Combination of the above creates a multitude of possible (apparent) issues on Pull/Push:

• pulling a beam, moving its driving curve up/down and pushing back will result in no change because of it being overconstrained by the previously pulled parameters being pushed back
• pulling a level, using SetProperty on its Elevation and pushing back to Revit will result in no change because first the Revit element will be updated by the property, but then changed back by the previously pulled elevation parameter that stayed unchanged
• (...)

TL;DR:

The current approach to pulling and pushing Revit parameters does not work because always all of them get pulled and pushed, causing various interferences with BHoM object properties.

Proposed solution:

• on Read the location of BHoM object would be calculated to ensure that it represents the Revit element (e.g. BHoM representation of a Revit beam aligned to its top face would be moved by half of its height downwards due to the fact that BHoM beam does not carry offsets)
• later on Read all parameters would be stored in RevitParameters fragment
• the parameters to be overwritten on Push would be stored in BHoM object's CustomData (therefore would need to be explicitly added by the user before pushing)
• on Create the location of a Revit element would be calculated to ensure that it represents the BHoM object, taken into account the modifications of Revit element imposed by parameters pushed from CustomData (inverse of first point)
• on Update the location of a Revit element would be calculated to ensure that it represents the BHoM object, taken into account the modifications of Revit element imposed by parameters pushed from combination of CustomData and parameters of the element being updated (same as above)

This brings us to the following workflow for the end user:

The above will require the user to make an explicit request to change a chosen parameter, while still exposing all parameters on Pull (very useful) and leaving the Push formula fully open (anything can changed, conflicts explained at the beginning can be imposed by the end user if she/he likes).

I am rather against using a Fragment for pushed parameters because it will be yet another step to take for the user. Simple SetProperty will be way more intuitive I believe.

Other considered solutions:

• disallowing certain parameters from being overwritten - a straight road down to hell, identifying the ones and making sure they are well maintained would be a disaster + opennes and hackability would be limited (against BHoM principle)
• adding a switch to RevitPushConfig to push or not push parameters - this would introduce an everything or nothing approach that would not resolve the primary issue of some parameters being mishandled (still all of them would be pushed, including the troublemakers)
• adding an enum on RevitPushConfig to decide whether a BHoM object or its CustomData is meant to push - above issue with everything or nothing still valid + disastrous UX (why is it not pushing parameters any more?)

Please feel free if anything above is unclear or needs more visual explanation. Happy to push this one a bit as I would like to give it a first shot next week @al-fisher @rwemay

Thanks!

[kThorsager]

Well the current situation sounds terrifying and I like the new proposal a lot more as it seems to bring a lot of control back to the user.

And suggestion, not sure if possible, but Revit seems to store its parameters in blocks: (I'm surely making a fool out of myself as that's not the object definition but I hope it's structured similarly) Could the BHoM objects read by the adapter have its Revit Fragments segmented into these blocks? Then one would have a nice set of fragments where each define something which may or may not have been placed on the BHoM objects properties, and one could add a method which takes a fragment and sets its properties as custom-data. Guess it kind of depends on if said blocks exists and if they actually influence a well defined part of the Revit object, but think it would alleviate transferring a chunk of data between pull and push from fragment to CustomData.

The other more general thing is if CustomData is the place to store the information to override in Revit, seems more appropriate with a dedicated fragment. But I do realise the difficulties with doing that change without breaking every Revit script there is.

Either way, sound great to my ears

[pawelbaran]

Good point @kThorsager about scraping more info about the parameters. Not only their functional groups (which I think is possible), but also original units, type (length vs angle vs mass etc.).

Concerning your second remark, I would not be too concerned about breaking the existing scripts (he he) because they will break anyways (CustomData moved to fragment), I was more thinking of future scripts and making the process as user friendly as possible. Happy to discuss that one further though.

[al-fisher]

Good point @kThorsager about scraping more info about the parameters. Not only their functional groups (which I think is possible), but also original units, type (length vs angle vs mass etc.).

Agreed on that. ^^

And so then with any refactoring of this approach I do think we should take the opportunity to formalise as Revit Fragments. Will make clearer and also a chance to mirror more closely Revit structure as suggested above. Also as we are finding with pulling some objects - getting some monster long list of parameters in custom data - so good to make more modular

A general note - good clear issue and explanation above! Thanks for taking the time and care 😄

[rwemay]

This seems very clear and a good way forward thanks @pawelbaran . Will be great to get parameters into a formal fragment - will it be immutable? Also agree with @kThorsager ’s point - getting the parameter attributes. We should consider similar in our BHoM parameter mapping.

[pawelbaran]

Parameter mapping on Pull will be introduced before this issue is tackled, Push is likely to be a part of this one @rwemay.

I feel there is one question that has not been answered explicitly yet: Do we want to wrap parameters on Push into a Fragment?

Pros:

• neater on the code level
• probs more magic could be done with it in the long term

Cons:

• all existing scripts will break (not too desperate about that, there will be a few other breaking changes)
• UX - single-click SetProperty is super neat, while adding another component, say SetParameter is likely to cause a lot of confusion

Personally I prefer keeping pushed parameters in CustomData, but not sure if that will be sustainable enough.

[rwemay]

That could be more scalable, and we should be able to make use of similar/more detailed structure of parameters. It would also hopefully remove the 'dot' issue - the Revit parameter names could be string properties rather than keys. (People may use the push customdata for other workflows, not just pushing up to Revit). Could it be as simple as a system wide Parameters fragment that inherits from IBHoMParamters (as would RevitParameters fragment which would be IImutable). It would then be SetProperty with "Parameters" input.

[pawelbaran]

String properties will not work I'm afraid @rwemay - this would mean they would need to be predefined in the class?

What comes to my head now is 2 Fragments:

• PullParameters - generated on Pull, neved to be pushed back to Revit
• PushParameters - they could possibly be a dummy object with a dictionary that could be managed similarly to CustomData, i.e. SetProperty would add/modify a KeyValuePair in it - by these means we would be able to do SetProperty directly on an object with propertyName == "PushParameters.MyParam" to add a parameter to push with a single click

Or am I confusing things?

[rwemay]

I think the idea of having a 'protected' (Imutable) set of parameters that's distinct and come from a pull is sound. Not sure about PullParameters - sounds too much like the parameters of a pull itself (like a config), PulledParameters might be better. We would only be limited on names if we use a dictionary/key approach rather than a formal Parameter class wouldn't we? On push parameters, again, I think we potentially want something a bit more formal.

Just as a visual reference/reminder for us, I'm copying below a snapshot from a Revit SP file. Not suggesting we mirror this, but this is what we're needing/wanting to support/map across to.

[pawelbaran]

Yes, PulledParameters sounds much better!

Yes and no (🙊) about adding more information to the pushed parameters (if I understand correctly what you mean by pasting a snip from SP file) - I do not think that adding any more complexity to ParametersToPush would bring added value. To make a successful push we only need to specify the name of param and value, all the rest is redundant:

• polymorphism of parameter names will be managed by ParameterSettings
• units and values are handled by the in-built convert system

So personally I would be perfectly fine with a simple Dictionary<string, object> representing a set of params to push.

Btw, maybe ParametersFromRevit and ParametersToRevit?