Revit_Core_Engine: Diffing MEP elements failed due to location curve split

[vietle-bh]

I noticed that diffing MEP elements such as ducts would often fail. This is because things like a duct will possibly be exported as multiple objects with the same Guid to handle the case below.

https://github.com/BHoM/Revit_Toolkit/blob/15cfcd06a0f99ac02691f29ff175932ec595cf93/Revit_Core_Engine/Query/LocationCurve.cs#L163-L169

In this scenario, a diff will return null at this check... https://github.com/BHoM/BHoM_Engine/blob/bdde55560efd5460930a858b4a5c1c9392f893e0/Diffing_Engine/Compute/DiffWithCustomIds.cs#L110-L111

... which means Revit isn't too happy:

Should we make it so all diffing called from Revit have diffingConfig.AllowDuplicateIds set to True? Or, will we never diff MEP elements so this shouldn't be an issue?

[travispotterBH]

@kayleighhoude I think this leads a bit into our discussion on physical/analytical duct objects, and that we may want to make sure diffing is only on physical ducts objects once we've gotten the objects fully defined.

[pawelbaran]

Also good to ask @alelom about diffingConfig.AllowDuplicateIds - is it a good practice to turn it to true?

[alelom]

Also good to ask @alelom about diffingConfig.AllowDuplicateIds - is it a good practice to turn it to true?

It's definitely not a good practice to have duplicate Ids. If you allow them, how do you match objects from the past and current set? Only one object with duplicate ID will be picked, and you won't know which one. The problem is that you need to be able to reliably use the ID as the "link" between past and present.

Now, I do not know how the duplicates form in this specific case. You could think of experimenting with toggling diffingConfig.AllowDuplicateIds to true and see if you get acceptable results for this specific case. But I would not keep it toggled to true if I were you; rather, I'd switch it only when a specific case that justifies this action is identified by your program.

Is there another ID that does not incur in duplicates for these kind of objects? Remember that you can set a different ID to be used via the RevitDiffing's RevitIdName input.

[pawelbaran]

I think the source of duplicates is that a single Revit element is converted into a few BHoM objects (not going into details, it is valid in some contexts, where structures have it sorted and MEP is still working on that). So I think the right move now is to sort out the MEP object model and how does it map against Revit elements before we actually build more complex workflows based on the adapter. What do you think @travispotterBH @kayleighhoude?

[alelom]

I think the source of duplicates is that a single Revit element is converted into a few BHoM objects (not going into details, it is valid in some contexts, where structures have it sorted and MEP is still working on that). So I think the right move now is to sort out the MEP object model

If that's the case, yes I would agree. Ideally you would need to have a mapping function that takes the ID of the single MEP Elements and returns a List of IDs, so each resulting ID can be associated with a single BHoM Object. The single ID ideally should be formed in way that allows converting it and its siblings back to a single one. For example, by appending a progressive number to the MEPElement's ID: [MEPElementID]-01, [MEPElementID]-02...

[travispotterBH]

@alelom @pawelbaran I like the idea of appending a progressive number to the MEPElement's ID. We are taking a look at this as part of BHoM/BHoM Issue: https://github.com/BHoM/BHoM/issues/1403

While we haven't figured out exactly what we want to do, we are considering an approach that allows us to specify when we'd like the broken curve (multiple elements representing one real Revit Element) and when we'd like only the actual Revit Element. This is likely to be through the action config selection of a MEPPhysical and MEPEnvironmental discipline selection.

There are some cases, such as in our pressure drop calculation that we will actually need both representations. For a duct pressure drop over a straight run with taps, the pressure drop over each section of the straight run reduces with the reduction in flow. So we will calculate that pressure drop, sum it across the broken curve elements and assign it to the real (non-broken) Revit element.

[pawelbaran]

@travispotterBH that sounds good - however, please note that we should minimise the number of new disciplines, so personally I would recommend using Physical for your 1:1 mapping and a new one, MEP for the analytical pulls.

[travispotterBH]

oh, for sure. As I wrote that I knew that I should have stated it the way you just did 😄

[travispotterBH]

Branch for latest code effort here: https://github.com/BHoM/Revit_Toolkit/tree/DuctElementsDisciplineCreateMethods

Closing related PR as it will not be accomplished in this sprint.