Fix C# bindings - unbreak the build and add missing partial classes #5377

jmarrec · 2025-03-20T15:17:27Z

Pull request overview

Fixes C# bindings build due to:
- Fix #5355 - E+ 25.1.0: Wrap OutputControl:ResilienceSummaries #5365
- Wrap PythonPlugin:SearchPaths #5312
Add missing partial classes for c#

Alright after Add workflow_dispatch to C#/python bindings workflows, so we can at least periodically test them #5376 , I launched a C# build which exhibits the issue: https://github.com/NREL/OpenStudio/actions/runs/13972812795/job/39118825152#step:5:4930

Pull Request Author

Model API Changes / Additions
Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
Model API methods are tested (in src/model/test)
EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
All new and existing tests passes
If methods have been deprecated, update rest of code to use the new methods

Labels:

If change to an IDD file, add the label IDDChange
If breaking existing API, add the label APIChange
If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
Labeling is ok
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
If feature, test running new feature, try creative ways to break it
CI status: all green or justified

jmarrec · 2025-03-20T15:53:29Z

Launching a C# workflow on this branch: https://github.com/NREL/OpenStudio/actions/runs/13973672884

jmarrec · 2025-03-20T16:47:35Z

My change is good, that error is gone, but C# bindings are still broken

All projects are up-to-date for restore.
/media/DataExt4/Software/Others/OS-build-csharp2/csharp_wrapper/generated_sources/OpenStudioModelResources/OpenStudioModelResources.cs(29,76): error CS1503: Argument 1: cannot convert from 'OpenStudio.EnergyManagementSystemCurveOrTableIndexVariable' to 'OpenStudio.Model' [/media/DataExt4/Software/Others/OS-build-csharp2/csharp_wrapper/OpenStudio.csproj]

jmarrec · 2025-03-20T16:51:30Z

It got beef with

OpenStudio/src/model/ModelResources.i

Lines 337 to 341 in 0f14dd3

    
           // Overloaded Ctor, calling Ctor that doesn't use Curve 
        
           public EnergyManagementSystemCurveOrTableIndexVariable(Model model, OpenStudio.Curve curve) 
        
             : this(model) { 
        
             this.setCurveOrTableObject(curve); 
        
           }

Not sure what's up, will try to get to it tomorrow

jmarrec · 2025-03-20T16:51:45Z

(hit the wrong button sorry)

jmarrec · 2025-04-02T07:33:14Z

Oh ok, actually the issue is not where I thought it was, it's here:

OpenStudio/src/model/ModelResources.i

Lines 343 to 345 in a882f64

    
           public OptionalPythonPluginSearchPaths pythonPluginSearchPaths() { 
        
             return OpenStudio.OpenStudioModelResources.pythonPluginSearchPaths(this); 
        
           }

This happened when wrapping PythonPluginSearchPaths. It got added it the wrong location

fe3461a#diff-b21ac9f937ee3830c707b3af186bb6ba53cc7a0b43fd114df09b071ed471adf7R343

jmarrec · 2025-04-02T07:48:19Z

Alright, locally it builds fine. Launching a C# CI run at https://github.com/NREL/OpenStudio/actions/runs/14214305608

jmarrec · 2025-04-02T09:07:11Z

Alright, working on CI. We have new missing partial classes though, I guess it's time to fix it.

Remember https://github.com/NREL/OpenStudio/blob/a882f64d0989414b26e18dc071415740ee92c43c/developer/ruby/FindMissingSWIGTypes.rb ?

@TShapinsky

Not sure why it was excluded entirely, maybe just a typo? @TShapinsky FYI Last one standing is std::string_view, cf #5393

jmarrec · 2025-04-02T10:26:57Z

src/alfalfa/Alfalfa.i

 %}

-%ignore openstudio::alfalfa::AlfalfaComponentBase;
+%ignore openstudio::alfalfa::AlfalfaComponentBase::clone;


@TShapinsky not sure why it was entirely ignored, or if it was just a typo?

Not ignoring it doesn't mean you can instantiate it.

In [2]: openstudio.alfalfa.AlfalfaComponentBase() --------------------------------------------------------------------------- AttributeError Traceback (most recent call last) Cell In[2], line 1 ----> 1 openstudio.alfalfa.AlfalfaComponentBase() File /media/DataExt4/Software/Others/OS-build-release2/Products/python/openstudioalfalfa.py:141, in AlfalfaComponentBase.__init__(self, *args, **kwargs) 140 def __init__(self, *args, **kwargs): --> 141 raise AttributeError("No constructor defined - class is abstract") AttributeError: No constructor defined - class is abstract

jmarrec · 2025-04-02T10:27:17Z

src/model/ModelZoneHVAC.i

+#if defined(SWIGCSHARP) || defined(SWIGJAVA)
+  %inline {
+    namespace openstudio {
+      namespace model {
+
+        // AirCondVRF, reimplemented from ModelHVAC.i
+        std::vector<ZoneHVACTerminalUnitVariableRefrigerantFlow> terminals(const openstudio::model::AirConditionerVariableRefrigerantFlowFluidTemperatureControl& airCondVRF) {
+          return airCondVRF.terminals();
+        }


An example of missing partial classes

jmarrec · 2025-04-02T10:29:33Z

@MingboPeng @arif-hanif the test step is failing, I wonder if you can help, as I don't understand the C# ecosystem enough to be honest.

I think this is just because GHA (or the default .NET installer for that matter) updated their default .NET to be 8.0.0 and above, and we try to test something targeting .NET 7.0

/Users/runner/.dotnet/sdk/9.0.202/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(254,5): warning NETSDK1201: For projects targeting .NET 8.0 and higher, specifying a RuntimeIdentifier will no longer produce a self contained app by default. To continue building self-contained apps, set the SelfContained property to true or use the --self-contained argument. [/Users/runner/work/OpenStudio/OpenStudio/csharp/examples/OpenStudio.Tests/OpenStudio.Tests.csproj::TargetFramework=NET7]
  OpenStudio.Tests -> /Users/runner/work/OpenStudio/OpenStudio/csharp/examples/OpenStudio.Tests/bin/x64/Debug/net7/osx-x64/OpenStudio.Tests.dll
Test run for /Users/runner/work/OpenStudio/OpenStudio/csharp/examples/OpenStudio.Tests/bin/x64/Debug/net7/osx-x64/OpenStudio.Tests.dll (.NETCoreApp,Version=v7.0)
VSTest version 17.13.0 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
Testhost process for source(s) '/Users/runner/work/OpenStudio/OpenStudio/csharp/examples/OpenStudio.Tests/bin/x64/Debug/net7/osx-x64/OpenStudio.Tests.dll' exited with error: You must install or update .NET to run this application.
App: /Users/runner/work/OpenStudio/OpenStudio/csharp/examples/OpenStudio.Tests/bin/x64/Debug/net7/osx-x64/testhost.dll
Architecture: x64
Framework: 'Microsoft.NETCore.App', version '7.0.0' (x64)
.NET location: /Users/runner/.dotnet/
The following frameworks were found:
  8.0.1 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App]
  8.0.4 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App]
  8.0.7 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App]
  8.0.14 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App]
  9.0.1 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App]
  9.0.3 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App]
Learn more:
https://aka.ms/dotnet/app-launch-failed
To install missing framework, download:
https://aka.ms/dotnet-core-applaunch?framework=Microsoft.NETCore.App&framework_version=7.0.0&arch=x64&rid=osx-x64&os=osx.13
. Please check the diagnostic logs for more information.

https://github.com/NREL/OpenStudio/actions/runs/14214305608/job/39834389714#step:5:66

jmarrec · 2025-04-02T10:30:28Z

Actually the question might be whether we shouldn't bump the target from .NET 7 to .8 or above.

Seems like it's EOL https://dotnet.microsoft.com/en-us/download/dotnet/7.0/runtime?cid=getdotnetcore&os=macos&arch=x64

jmarrec · 2025-04-02T10:42:30Z

Ok I found where this is defined:

OpenStudio/csharp/examples/OpenStudio.Tests/OpenStudio.Tests.csproj

Line 4 in a882f64

jmarrec · 2025-04-02T10:44:35Z

is this ok though?

OpenStudio/csharp/developer/OpenStudio/OpenStudio.csproj.in

Lines 3 to 4 in a882f64

    
           <TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">NET45;netstandard2.0</TargetFrameworks> 
        
           <TargetFramework Condition="'$(OS)' != 'Windows_NT'">netstandard2.0</TargetFramework>

That seems awfully outdated?

Are there any gains for bumping it to NET8 for eg?

…HA only has NET8 and 9 currently) The alternative is to use https://github.com/actions/setup-dotnet

jmarrec · 2025-04-02T10:48:35Z

csharp/examples/OpenStudio.Tests/OpenStudio.Tests.csproj


  <PropertyGroup>
-    <TargetFrameworks>NET7</TargetFrameworks>
+    <TargetFrameworks>NET7;NET8;NET9;NET10</TargetFrameworks>


This is my attempt at fixing the C# test step, just allow whatever is on the GHA runner.

Launch a run at https://github.com/NREL/OpenStudio/actions/runs/14217768843

I thought this would allow testing with ANY version installed in that list, but it seems that it tries to do ALL of them so the workflow is probably going to fail.

Determining projects to restore... /usr/lib/dotnet/sdk/8.0.114/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.TargetFrameworkInference.targets(166,5): error NETSDK1045: The current .NET SDK does not support targeting .NET 9.0. Either target .NET 8.0 or lower, or use a version of the .NET SDK that supports .NET 9.0. Download the .NET SDK from https://aka.ms/dotnet/download [/media/DataExt4/Software/Others/OS-build-csharp2/Testing/csharp/OpenStudio.Tests/OpenStudio.Tests.csproj::TargetFramework=NET9]

MEH, just going to use NET8 and keep move on for now.

Launched at https://github.com/NREL/OpenStudio/actions/runs/14218506478

Ok that run worked, including testing.

ci-commercialbuildings · 2025-04-02T11:02:01Z

CI Results for 92336a6:

Ubuntu 24.04:
- OpenStudio-3.10.0-alpha+92336a647c-Ubuntu-24.04-x86_64.deb
- OpenStudio-3.10.0-alpha+92336a647c-Ubuntu-24.04-x86_64.tar.gz
Ubuntu 22.04:
Darwin:
- OpenStudio-3.10.0-alpha+92336a647c-Darwin-x86_64.dmg
- OpenStudio-3.10.0-alpha+92336a647c-Darwin-x86_64.tar.gz
Windows:
Code Coverage:

…kip ci]

MingboPeng · 2025-04-02T14:52:37Z

Hi @jmarrec, yes, Github Action disabled the .Net 7 by default. Just updating the test project to .Net 8 will just be fine.

jmarrec · 2025-04-02T15:46:18Z

@joseph-robertson A quick look from you would be nice please. It's ready to drop in.

jmarrec · 2025-04-02T15:47:11Z

is this ok though?

OpenStudio/csharp/developer/OpenStudio/OpenStudio.csproj.in

Lines 3 to 4 in a882f64

<TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">NET45;netstandard2.0</TargetFrameworks>

<TargetFramework Condition="'$(OS)' != 'Windows_NT'">netstandard2.0</TargetFramework>

That seems awfully outdated?

Are there any gains for bumping it to NET8 for eg?

@MingboPeng is there a gain to be had, or is it better to keep targeting old versions so it's compatible with pretty much anything?

MingboPeng · 2025-04-02T16:14:45Z

Hi @jmarrec,
netstndard2.0 will be fine for latest version of .NetCore.

https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-0

Copilot

Copilot reviewed 1 out of 13 changed files in this pull request and generated no comments.

Files not reviewed (12)

csharp/examples/OpenStudio.Tests/OpenStudio.Tests.csproj: Language not supported
csharp/examples/OpenStudio.Tests/OpenStudio.Tests.x86.csproj: Language not supported
src/alfalfa/Alfalfa.i: Language not supported
src/model/ModelCore.i: Language not supported
src/model/ModelGeometry.i: Language not supported
src/model/ModelHVAC.i: Language not supported
src/model/ModelResources.i: Language not supported
src/model/ModelZoneHVAC.i: Language not supported
src/utilities/filetypes/Filetypes.i: Language not supported
src/utilities/filetypes/WorkflowStep.cpp: Language not supported
src/utilities/filetypes/WorkflowStep.hpp: Language not supported
src/utilities/xml/XMLValidator.i: Language not supported

jmarrec added Developer Issue component - C# labels Mar 20, 2025

jmarrec self-assigned this Mar 20, 2025

jmarrec closed this Mar 20, 2025

jmarrec reopened this Mar 20, 2025

Hotfix C# bindings for #5365

180df4e

jmarrec force-pushed the hotfix/5365 branch from d821507 to 180df4e Compare April 1, 2025 15:16

jmarrec changed the title ~~Hotfix C# bindings for #5365~~ Hotfix C# bindings for #5365 and #5312 Apr 2, 2025

Fix C# bindings for #5312

f50acda

jmarrec mentioned this pull request Apr 2, 2025

Update to latest SWIG so we can use std_string_view.i #5393

Open

Add missing Partial Classes for EMSActuator

f3e5d52

jmarrec added 7 commits April 2, 2025 11:17

Add missing partial classes for AirCondVRF to add/remove/list terminals

94cebdc

Add missing partial classes for ExternalFile::chillerElectricASHRAE205s

84038b4

Add missing template for OptionalEpwGroundTemperatureDepth

b8f1feb

C# bindings for WorkflowJSON::getMeasureStepsWithIndex

6c1d417

XmlValidator: import LocalBCL for BCLXMLType is swigged properly

79e8d64

Partial classes for Space related to ZoneMixing getters

3d1f3fd

Ignore protected Impl ctors from swig type things

789d107

jmarrec added the Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. label Apr 2, 2025

last missing swig type I can fix: AlfalfaComponentBase

1c2ab98

Not sure why it was excluded entirely, maybe just a typo? @TShapinsky FYI Last one standing is std::string_view, cf #5393

jmarrec changed the title ~~Hotfix C# bindings for #5365 and #5312~~ Fix C# bindings - unbreak the build and add missing partial classes Apr 2, 2025

jmarrec commented Apr 2, 2025

View reviewed changes

Try fixing the C# test step by allowing NET7, NET8, NET9 and NET10 (G…

92336a6

…HA only has NET8 and 9 currently) The alternative is to use https://github.com/actions/setup-dotnet

jmarrec commented Apr 2, 2025

View reviewed changes

Whatever - set the C# test project to use NET8 and be done with it [s…

ecc90ca

…kip ci]

jmarrec requested review from Copilot and joseph-robertson April 7, 2025 08:59

Copilot AI reviewed Apr 7, 2025

View reviewed changes

jmarrec merged commit 8abc1f6 into develop Apr 25, 2025
13 checks passed

jmarrec deleted the hotfix/5365 branch April 25, 2025 11:53

Fix C# bindings - unbreak the build and add missing partial classes #5377

Fix C# bindings - unbreak the build and add missing partial classes #5377

Uh oh!

Conversation

jmarrec commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

jmarrec commented Mar 20, 2025

Uh oh!

jmarrec commented Mar 20, 2025

Uh oh!

jmarrec commented Mar 20, 2025

Uh oh!

jmarrec commented Mar 20, 2025

Uh oh!

jmarrec commented Apr 2, 2025

Uh oh!

jmarrec commented Apr 2, 2025

Uh oh!

jmarrec commented Apr 2, 2025

Uh oh!

jmarrec Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

jmarrec Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

jmarrec commented Apr 2, 2025

Uh oh!

jmarrec commented Apr 2, 2025

Uh oh!

jmarrec commented Apr 2, 2025

Uh oh!

jmarrec commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmarrec Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

jmarrec Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

jmarrec Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

jmarrec Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

ci-commercialbuildings commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MingboPeng commented Apr 2, 2025

Uh oh!

jmarrec commented Apr 2, 2025

Uh oh!

jmarrec commented Apr 2, 2025

Uh oh!

MingboPeng commented Apr 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmarrec commented Mar 20, 2025 •

edited

Loading

jmarrec commented Apr 2, 2025 •

edited

Loading

ci-commercialbuildings commented Apr 2, 2025 •

edited

Loading