Skip to content

Add BHoM Filters for optimisation on Excel Add-in #1532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

linhnam-nguyen
Copy link
Contributor

@linhnam-nguyen linhnam-nguyen commented Feb 7, 2025

NOTE: Depends on

Require PR: Excel UI -> BHoM/Excel_UI#406
New dependencies to Revit_Engine, Nuget Packages:

  • Microsoft.CodeAnalysis
  • Microsoft.CodeAnalysis.CSharp

Objectif

To create a comprehensive set of tools for BIM managers, enabling:

  • Free exploration of models with highly flexible data manipulation capabilities.
  • The creation of simple, communication-friendly spreadsheets for project managers.

This toolkit is designed for BIM managers with some coding and Excel skills, reducing reliance on third-party software. It allows for faster development of new in-house toolboxes without the need for extensive software development expertise. Examples of potential application:

  • Externalize geometry constraints into an Excel file, centralizing family datasheets and lookup tables in a single location. This facilitates standardization and prevents the creation of unnecessary parameters within families, leading to cleaner, more efficient Revit files with a smaller footprint.
  • Establish constraints among parameters of System Family elements like ducts and pipes, enabling custom calculations and verification methods tailored to specific needs.
  • Automate classification processes, perform lookups, and assign elements based on conditions, significantly streamlining the takeoff process or documentation and improving overall productivity.
  • Out-source and manage information of equipment's data to a spreadsheet, promote use of generic objects for early stage of design, this will facilitate project manager or engineers to focus on their research and design, rather than manipulation or tracking metadata model.

Test files

Excel and Revit (french version) test files:
Test File.zip

Changelog

Feature: ParameterFormula

  • Add IsInstanceParameter method to check on a RevitParameter of a BHoM object,
  • Add LabelForSymbolTypeId method to show display unit of RevitParameter in BHoM object
  • Add new properties to RevitParameter: IsInstance, DisplayUnit
  • Create a new BHoM object ParameterFormula to hold relationships among the Revit Parameter
  • Add CreateFormula methods to compile in runtime and return a delegate from a built-in Templates
  • Add ApplyFormula methods to calculate ParameterFormula and transfer results to RevitParameter objects
  • Add GetRevitParameter method to retrieve a RevitParameter object of a BHoM entity.

Features: New methods to RevitEngine project to improve Excel UX:
Reason for this proposal: Reduce intensive pull requests operation needs for Excel user therefore more responsive work on large models

  • Add GetRevitParameterValues to investigate on a List of BHoM objects to reduce compute need and for a better organized spreadsheet.
  • Add FilterUniqueRevitElements : In case of MEPCurve, an object might be segmented into small segments while RevitParameter values unchanged and causing incorrect takeoff or aggregation.
  • Add FilterByRevitParameter and FilterByRevitParameters that can handle multiple types of filters, based on IRevitParameterFilter.
  • Add SortByRevitParameter to organize pulled objects.

Issues may happen:

These developments are working on Windows10 and Windows 11 machines with Revit version 2023:
CreateFormula may not work because of cached assemblies of compiler, worked solution for me:
https://learn.microsoft.com/en-us/answers/questions/1302681/could-not-load-file-or-assembly-system-runtime-com

Example of spreadsheet design:

A Settings Sheet to control all requests:
image
We can configure a spreadsheet to pull all necessary objects at once for a working session:
image
GetRevitParameterValues makes it able to use one formula cell per column. By using Excel array syntax for dynamic array references, the spreadsheet is optimized for startup time and calculation.
image
Pivot table is a useful feature to take a snapshot of pulled data. **Data is only renewed on demand. **
image
image

Perform a bulk parameter query to avoid creating multiple lines in Excel, thereby improving the plugin’s performance in Excel.
…pecified Revit parameter. To be implemented:

- ApplyFormula on a category
- IsTypeParameter check
- IsInstanceParameter check
- GenerateMethod
Add DisplayUnit to RevitParameter, DisplayUnit will show unit symbol to UI.
The Feature is debugging.
@linhnam-nguyen linhnam-nguyen changed the title Feature: Parameters bindings and UX improvements for Excel Add-ins Add BHoM Filters for optimisation on Excel Add-in Feb 20, 2025
@linhnam-nguyen linhnam-nguyen marked this pull request as draft February 20, 2025 09:56
@linhnam-nguyen
Copy link
Contributor Author

Hi @pawelbaran, I've just come across your recently merged commits in Revit Toolkit and BHoM Engine.

I somewhat understand your intention to unify filters in requests and queries by using ICondition for filtering parameter values in Revit Toolkit. However, regarding FilterByParameterExistence, is this a work in progress with a new approach, or is it being removed from the Toolkit?

In my current workflow, I use this feature to distinguish between instance and type elements based on the Phase Created parameter. If there’s another way to achieve this, I’d love to learn about it. Otherwise, if it makes sense, I’m considering reimplementing it using ElementIdsByCondition—though I’m unsure if that’s the best approach, since FilterByParameterExistence isn’t a ConditionRequest.

Regarding FormulaCondition, I really like the lean approach you’ve taken—it makes formulas much more readable and intuitive. However, in my case, I see a few challenges unless the Solve method is modified. Please correct me if I’m mistaken—I’m still learning, but here’s what I’ve noticed:

  • The syntax supports basic mathematical operators and if/else logic, which is great.
  • Methods from System.Math, string operations, and LINQ expressions don’t seem to be supported at the moment.
  • When testing runtime code execution using CSharpScript in my prototype, I ran into memory leak issues due to the lack of GC cleanup for assemblies ( Every CSharpScript.EvaluateAsync() call generates a new assembly that does not get unloaded dotnet/roslyn#41722 ). I worry this could become unsustainable for large models with heavy calculations.
  • Performance concern: Since formulas are re-evaluated on each iteration, there might be performance issues if there’s no caching mechanism in place, so that I use Roslyn Emit method on memory with a cache mechanism.

I’d love to hear your thoughts on this and see if there are ways to improve flexibility and optimize performance. I also sent you a LinkedIn request in case that’s easier to discuss in details. Let me know what works for you.

Looking forward to your insights.

@pawelbaran
Copy link
Member

Thanks for the response @linhnam-nguyen, this is all really valuable! Answering to the key points you mention:

I somewhat understand your intention to unify filters in requests and queries by using ICondition for filtering parameter values in Revit Toolkit. However, regarding FilterByParameterExistence, is this a work in progress with a new approach, or is it being removed from the Toolkit?

This object is still in the oM, not to be deleted in this milestone, but I am on the fence whether not to replace it with a ConditionRequest holding BH.oM.Verification.Conditions.HasValue. This, however, would return same results for both empty parameter and no parameter, therefore I am still not sure if this is the right approach - happy to hear your opinion, could help in making the final decision 👍

In my current workflow, I use this feature to distinguish between instance and type elements based on the Phase Created parameter. If there’s another way to achieve this, I’d love to learn about it. Otherwise, if it makes sense, I’m considering reimplementing it using ElementIdsByCondition—though I’m unsure if that’s the best approach, since FilterByParameterExistence isn’t a ConditionRequest.

There definitely are other ways to achieve this, for example adding IsElementType(this Element element) extension method, which then could be called when processing ValueCondition with value source being PropertyValueSource:

ValueCondition condition = new ValueCondition
{
    ValueSource = new PropertyValueSource
    {
        PropertyName = "IsElementType"
    },
    ReferenceValue = false,
    ComparisonType = ValueComparisonType.EqualTo
}

Getting the output of an extension method as a property value is possible thanks to the following line:
https://github.com/BHoM/BHoM_Engine/blob/052e1e96e496178ef0a47ffc921fbd513cbc3274/Verification_Engine/Compute/TryGetValueFromSource.cs#L288

  • The syntax supports basic mathematical operators and if/else logic, which is great.

correct

  • Methods from System.Math, string operations, and LINQ expressions don’t seem to be supported at the moment.

correct

Wow that is interesting! I have not heard of that issue and never faced it when doing my own testing, nor writing a simple app on top of the formula mechanism. Is there a chance you could share a console app that would highlight the issue? Naturally it should be fixed.

  • Performance concern: Since formulas are re-evaluated on each iteration, there might be performance issues if there’s no caching mechanism in place, so that I use Roslyn Emit method on memory with a cache mechanism.

Agreed, performance is far from great atm, is there a chance you could shed a bit more light on how are you using Roslyn Emit? As above, defo something to improve if possible 👍

Finally a few words about Solve method: it was written to enable evaluating simple, formula-based checks and reporting the results. I have a strong feeling it requires a lot of work before we get even remotely close to calling it finished. You are right that you will need to refactor it heavily for your purpose, therefore I mentioned it as a starting point or inspiration rather than something you could use off the shelf. Looks like you did at least as much research and testing around that topic as I did, so could be a good one for us to look into together to make the next iteration more flexible/scalable.

But first, would be great if we could narrow down the problem to formula evaluation - please give it a shot to build as much as you can using the objects and methods already available in BHoM, so that we could only focus on the gaps rather than the entire architecture at the same time. Cheers!

@linhnam-nguyen
Copy link
Contributor Author

Hi @pawelbaran, I´m back. Thank you for your response.

This object is still in the oM, not to be deleted in this milestone, but I am on the fence whether not to replace it with a ConditionRequest holding BH.oM.Verification.Conditions.HasValue. This, however, would return same results for both empty parameter and no parameter, therefore I am still not sure if this is the right approach - happy to hear your opinion, could help in making the final decision 👍

That´s great to hear.

There definitely are other ways to achieve this, for example adding IsElementType(this Element element) extension method, which then could be called when processing ValueCondition with value source being PropertyValueSource:

ValueCondition condition = new ValueCondition
{
    ValueSource = new PropertyValueSource
    {
        PropertyName = "IsElementType"
    },
    ReferenceValue = false,
    ComparisonType = ValueComparisonType.EqualTo
}

I’ll try it right away and see how it works in my case.

Wow that is interesting! I have not heard of that issue and never faced it when doing my own testing, nor writing a simple app on top of the formula mechanism. Is there a chance you could share a console app that would highlight the issue? Naturally it should be fixed.

The issue with memory leaks in CSharpScript has been consistent in my tests. You can try running this simple console app to analyze the issue:

using System.Reflection;
using Microsoft.CodeAnalysis;
using ConsoleApp1;
using Microsoft.CodeAnalysis.CSharp.Scripting;
using Microsoft.CodeAnalysis.Scripting;


public static partial class MethodGenerator
{
   public class FormulaGlobals
   {
       public Dictionary<string, object> Variables { get; set; } = new();

       public T Get<T>(string key)
       {
           if (Variables.TryGetValue(key, out var value))
           {
               return (T)Convert.ChangeType(value, typeof(T));
           }
           throw new KeyNotFoundException($"Variable '{key}' not found.");
       }
   }

   public static async Task<object> FormulaScripting(string codeString, Dictionary<string, object> variables = null)
   {
       var globals = new FormulaGlobals { Variables = variables ?? new Dictionary<string, object>() };

       var options = ScriptOptions.Default
           .AddReferences(
               typeof(object).Assembly,
               typeof(Console).Assembly,
               typeof(Dictionary<,>).Assembly,
               typeof(Enumerable).Assembly
           )
           .AddImports("System", "System.Linq", "System.Collections.Generic");

       return await CSharpScript.EvaluateAsync(codeString, options, globals, typeof(FormulaGlobals));
   }
}

class Program
{
   static async Task Main()
   {
       long memoryBefore = GC.GetTotalMemory(true);
       Console.WriteLine($"Memory Before: {memoryBefore / 1024} KB");

       for (int i = 0; i < 1000; i++)
       {
           string formula = "Get<int>(\"x\") + Get<int>(\"y\")";
           var variables = new Dictionary<string, object>
       {
           { "x", 10 },
           { "y", 20 }
       };
           object result = await MethodGenerator.FormulaScripting(formula, variables);
           GC.Collect(); // Force garbage collection
           long memoryAfter = GC.GetTotalMemory(true);
           Console.WriteLine($"Memory After {i} iterations: {memoryAfter / 1024} KB");
       }
       long memoryAfterGC = GC.GetTotalMemory(true);

       Console.WriteLine($"Memory After GC: {memoryAfterGC / 1024} KB");

   }
}

image
image

As you can see, I can not use GC collector on CScript. The memory usage keeps increasing.

  • Performance concern: Since formulas are re-evaluated on each iteration, there might be performance issues if there’s no caching mechanism in place, so that I use Roslyn Emit method on memory with a cache mechanism.

Agreed, performance is far from great atm, is there a chance you could shed a bit more light on how are you using Roslyn Emit? As above, defo something to improve if possible 👍

Regarding Roslyn emit, you can check out in my commit : Revit_Engine/Compute/CreateFormula.cs.

To summarize my approach:

  1. Formula Preparation: I inserted the user’s formula string into a predefined template, creating a static method inside a CustomMethod class. I included commonly used namespaces (such as System.Linq) to allow users to access built-in methods.
  2. Dynamic Input Handling: I stored variables as key-value pairs (Dictionary<string, object>) where users define both variable names and types. These were then converted into method parameters inside the generated code. This was inspired by your CSharpToolkit but implemented in a more direct and simplified way.
  3. Memory Compilation: The generated code is compiled in-memory and returned as a delegate for execution. This ensures that GC (Garbage Collector) can properly clean up resources, preventing memory leaks.
  4. Caching Mechanism: If compilation succeeded, the entire code string was stored in a cache dictionary (Dictionary<string, Delegate>) to avoid redundant compilation in future executions.

This approach gives power users complete flexibility, allowing them to define single-line or multi-line calculations. However, the downside is that debugging can be difficult, especially since BHoM lacks a robust logging mechanism. When testing in BHoM Excel, I also noticed that error messages are limited to 255 characters, which means longer debugging messages get truncated. I had to trim error logs to only essential details, which made troubleshooting more difficult.

But first, would be great if we could narrow down the problem to formula evaluation - please give it a shot to build as much as you can using the objects and methods already available in BHoM, so that we could only focus on the gaps rather than the entire architecture at the same time. Cheers!

Totally agree. I'm exploring ways to enhance your evaluation framework and refine the approach. Hopefully, we can find a successful path forward for this feature.

@linhnam-nguyen
Copy link
Contributor Author

Hi @pawelbaran,

I've recently updated the PR with the modifications we discussed. Here’s what changed in this update:

  • Removed the IsInstance property from RevitParameter,
  • Removed ParameterFormula (to be included in a future PR) ,
  • Adapted FilterByRevitParameters to work with ICondition objects and Verification mechanism.

I’m looking forward to your thoughts. I think this PR is now in a good state to be reopened for review.
Thanks!

@linhnam-nguyen linhnam-nguyen marked this pull request as ready for review April 10, 2025 09:28
Copy link
Member

@pawelbaran pawelbaran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @linhnam-nguyen for another push 👍

This time I flipped the roles a bit and branched off your PR to try and restructure your code how I would do it. The code is not perfectly rounded but should show my intent well enough - it would be great if you could have a look and tell if this direction would work for you or I went too far with my cleanup. Main changes I made:

  • renamed the properties in RevitParameter class to make them a bit easier to read for he users
  • removed out parameter from ParameterValue method, also reshuffled the label query methods
  • simplified TryGetValueFromSource method
  • simplified FilterByRevitParameter method and made it only accept ValueComparisonType enums as I do not think it is a good idea to do custom string parsing inside Revit_Toolkit - as a core repo it should stay clean of any customisation, so that part should be a part of the Excel workflow, and if that is not feasible then I would argue we need a new Engine project that would do this sort of juggling
  • replaced FilterUniqueRevitElements with a more generic Unique method

What is still hanging and requires another iteration:

  • GetRevitParameter vs GetRevitParameters vs GetRevitParameterValue vs GetRevitParameterValues seems to be a bit inconsistent in terms of returned results - some methods return property values and the others not
  • as commented before, I think GetRevitParameterValues should not be needed, it should be solved on the side of Excel_UI by enabling function calls against each item of the input collection (will chase it internally to check how feasible is that) - in the meantime, I would recommend you creating your personal repo that would contain an Engine project where you could dump all such utilities as well as prototype your workflows faster.

Please let me know how does it all sound - if you like the direction I took on my branch, I am happy to polish the code and raise a PR for you to review, meanwhile you could open a personal repo with utility methods to keep the momentum going.

@pawelbaran
Copy link
Member

@linhnam-nguyen please also have a look at BHoM/Excel_UI#333

@linhnam-nguyen
Copy link
Contributor Author

Hi @pawelbaran , and thank you for taking the time to go through my code.The improvements you made make a lot of sense, and I really appreciate the clarity you've brought in. Below are some of my remarks and clarifications regarding your modifications:

  1. Separation of DisplayUnit (or UnitLabel) from ParameterValue Your approach is much cleaner and more concise than mine. When I originally implemented it, I was trying to minimize the impact on the existing codebase and reduce direct access to the Revit API to help with performance.

  2. FilterByCondition / TryGetValueFromSource Your simplification is elegant, but it shifts more responsibility to the modeler. My initial goal was to make filtering more forgiving, allowing users to filter on either type or instance parameters, depending on what's available.

I was thinking of a scenario where users (or me) have many mechanical components from different manufacturers (with key info stored in type parameters). They may not know whether the data is stored on the type or instance level—they just want the results.

To support that flexibility, what do you think of introducing a third option like "any" or "both" for valueSource.FromType, using a nullable boolean? That way:

  • true = type only
  • false = instance only
  • null = allow both

Would love to hear your take on that!

  1. FilterUniqueRevitElements (Unique) I haven’t fully understood how this would now work. Could you show me an example of how to use it via the Grasshopper UI?

This function was originally written for MEP BHoM objects, where elements like pipes or ducts are segmented at each change in direction or cross-section—but the underlying Revit element remains the same. That creates confusion for users trying to synthesize quantities, so I needed a way to collapse those into unique Revit references.

  1. GetRevitParameterValues You’re right—this ties into a larger, open issue in the Excel_UI repo. It’s a deeper subject if we want the logic to scale in a truly intuitive and dynamic way.

That said, I found a workaround using Excel’s native lambda + array functions:
=MAP(A1:A4, LAMBDA(x, GetRevitParameterValue(x)))

This helps bypass the current limitations while still letting me apply the function across a range.

Since you’ve opened a fresh branch, I’d be happy to use this as a prototype and continue our discussion while you continue refining the review PR.

Thanks again! Looking forward to your thoughts.

@pawelbaran
Copy link
Member

@linhnam-nguyen all sounds good! I agree without any comments with everything not listed below 😉

  1. FilterByCondition / TryGetValueFromSource Your simplification is elegant, but it shifts more responsibility to the modeler. My initial goal was to make filtering more forgiving, allowing users to filter on either type or instance parameters, depending on what's available.

I was thinking of a scenario where users (or me) have many mechanical components from different manufacturers (with key info stored in type parameters). They may not know whether the data is stored on the type or instance level—they just want the results.

To support that flexibility, what do you think of introducing a third option like "any" or "both" for valueSource.FromType, using a nullable boolean? That way:

  • true = type only
  • false = instance only
  • null = allow both

Would love to hear your take on that!

This one is a bit tricky, personally I feel a bit against introducing this sort of ambiguity (always harder to manage/maintain), but I ran it against @michal-pekacki who agreed with your request, I am becoming outvoted 😢 However, assuming that it is you who sets up the spreadsheets for the rest of the office, couldn't you wrap the condition into two identical LogicalOrConditions where one takes param value from instance and the other from type? I guess as long as it works for simple conditions, it may not be perfect for the larger ones?

  1. FilterUniqueRevitElements (Unique) I haven’t fully understood how this would now work. Could you show me an example of how to use it via the Grasshopper UI?

Ahh sorry, I thought you want to cull out the exact duplicates from your collection - now I see what I proposed does not help. Let me think how to do it in an elegant/scalable way.

Since you’ve opened a fresh branch, I’d be happy to use this as a prototype and continue our discussion while you continue refining the review PR.

Feels like we're heading to an agreement 😄 I will try to fully convince myself about 2. and think about 3. - once that happens, I will finalise the work I did on my branch. Please tell me one thing: how urgent is this? If very, I will try to have a look later this week, otherwise it will easily go into the next one...

@linhnam-nguyen
Copy link
Contributor Author

linhnam-nguyen commented Apr 14, 2025

@pawelbaran , Please take your time—no urgency at all.
I’ve successfully built and deployed a local installation on our team’s machines, and the proposed features are already working well internally.
For me, this is more about making the ideas official so I can confidently share our spreadsheets and workflows back with the community. I genuinely feel like there’s a lot of untapped potential here—and perhaps we’re a bit underrated! 😊

@pawelbaran
Copy link
Member

Closing this PR as #1566 replaced it effectively.

@pawelbaran pawelbaran closed this Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants