-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add BHoM Filters for optimisation on Excel Add-in #1532
Conversation
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.
…rrors from Excel (255 characters)
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 In my current workflow, I use this feature to distinguish between instance and type elements based on the Regarding
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. |
Thanks for the response @linhnam-nguyen, this is all really valuable! Answering to the key points you mention:
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
There definitely are other ways to achieve this, for example adding 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:
correct
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.
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 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! |
Hi @pawelbaran, I´m back. Thank you for your response.
That´s great to hear.
I’ll try it right away and see how it works in my case.
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");
}
} As you can see, I can not use GC collector on CScript. The memory usage keeps increasing.
Regarding Roslyn emit, you can check out in my commit : Revit_Engine/Compute/CreateFormula.cs. To summarize my approach:
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.
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. |
Hi @pawelbaran, I've recently updated the PR with the modifications we discussed. Here’s what changed in this update:
I’m looking forward to your thoughts. I think this PR is now in a good state to be reopened for review. |
There was a problem hiding this 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 fromParameterValue
method, also reshuffled the label query methods - simplified
TryGetValueFromSource
method - simplified
FilterByRevitParameter
method and made it only acceptValueComparisonType
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 genericUnique
method
What is still hanging and requires another iteration:
GetRevitParameter
vsGetRevitParameters
vsGetRevitParameterValue
vsGetRevitParameterValues
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.
@linhnam-nguyen please also have a look at BHoM/Excel_UI#333 |
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:
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:
Would love to hear your take on that!
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.
That said, I found a workaround using Excel’s native lambda + array functions: 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. |
@linhnam-nguyen all sounds good! I agree without any comments with everything not listed below 😉
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
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.
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... |
@pawelbaran , Please take your time—no urgency at all. |
Closing this PR as #1566 replaced it effectively. |
NOTE: Depends on
Require PR: Excel UI -> BHoM/Excel_UI#406
New dependencies to Revit_Engine, Nuget Packages:
Objectif
To create a comprehensive set of tools for BIM managers, enabling:
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:
Test files
Excel and Revit (french version) test files:
Test File.zip
Changelog
Feature: ParameterFormula
IsInstanceParameter
method to check on a RevitParameter of a BHoM object,LabelForSymbolTypeId
method to show display unit of RevitParameter in BHoM objectRevitParameter
: IsInstance, DisplayUnitParameterFormula
to hold relationships among the Revit ParameterCreateFormula
methods to compile in runtime and return a delegate from a built-in TemplatesApplyFormula
methods to calculate ParameterFormula and transfer results to RevitParameter objectsGetRevitParameter
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
GetRevitParameterValues
to investigate on a List of BHoM objects to reduce compute need and for a better organized spreadsheet.FilterUniqueRevitElements
: In case of MEPCurve, an object might be segmented into small segments while RevitParameter values unchanged and causing incorrect takeoff or aggregation.FilterByRevitParameter
andFilterByRevitParameters
that can handle multiple types of filters, based on IRevitParameterFilter.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:





We can configure a spreadsheet to pull all necessary objects at once for a working session:
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.
Pivot table is a useful feature to take a snapshot of pulled data. **Data is only renewed on demand. **