SWIG C# fixes + dotnet support #3960

jmarrec · 2020-04-28T13:45:30Z

Pull request overview

This includes #3959 which are generally SWIG improvements for C#, and also adds dotnet support so one can build the C# bindings on Unix for eg.

Status:

Can build C# just fine on windows and Ubuntu
Can generate the nuget package on Ubuntu, I have trouble making it copy the DLLs to the build output folder of a c# project that adds it as a PackageReference
(When resolved the DLL location situation manually) I can use the C# bindings in another netcoreapp3.0 csproj
Added C# unit tests via xUnit, as a proof of concept.

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

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

Iteration

close #3713 (cherry picked from commit 09da50c)

….0-rc3

Updating master for 2.9.0 release

Updating to v2.9.1

cwstring.i is only implemented for python and tcl really.

… default constructor, so make one

Swig isn't happy if you do two typedef/templates for the same stuff (std::pair<std::string, std::string> here)

…e hiding inherited ```` Severity Code Description Project File Line Suppression State Warning CS0108 'MasslessOpaqueMaterial.iddObjectType()' hides inherited member 'ModelObject.iddObjectType()'. Use the new keyword if hiding was intended. [D:\OpenStudio\build-vs\csharp_wrapper\OpenStudio.csproj] csharp_sdk D:\OpenStudio\build-vs\csharp\generated_sources\OpenStudioModelResources\MasslessOpaqueMaterial.cs 81 ```

…ecated declarations to C#

…at causes multiple definitions

jmarrec

Wait for #3960 to go in first.

jmarrec · 2020-04-28T13:47:31Z

csharp/OpenStudio.nuspec.in

+    <file src="@OPENSTUDIO_CSHARP_DLLNAME@" target="lib" /> <!-- TODO: target should be lib\{framework}, eg `lib\netcoreapp3.0` -->
+    <file src="@OPENSTUDIO_CSHARP_LIBNAME@" target="build" /> <!-- target to 'build' is probably not right either -->
+    <file src="@OPENSTUDIO_MODEL_CSHARP_LIBNAME@" target="build" />
+    <file src="@OPENSTUDIO_TRANSLATORS_CSHARP_LIBNAME@" target="build" />
+    <file src="@OPENSTUDIO_MODEL_CSHARP_LIBNAME@" target="build" /> <!-- TODO why is this repeated? -->
+    <file src="@PROJECT_SOURCE_DIR@/csharp/OpenStudio.targets" target="build" /> <!-- To copy native binaries -->


openstudio_model_csharp.dll is added twice?

jmarrec · 2020-04-28T13:47:52Z

csharp/OpenStudio.targets

+    <Copy SourceFiles="@(NativeBinary)"
+          DestinationFiles="@(NativeBinary->'$(OutDir)\%(TargetPath)\%(Filename).so')"
+          Condition="'%(Extension)'=='.so'">
+      <Output TaskParameter="DestinationFiles" ItemName="FileWrites" />
+    </Copy>


Try support for *.so files on Unix.

jmarrec · 2020-04-28T13:49:05Z

csharp/CMakeLists.txt

+set(CSHARP_VERSION_PATCH ${PROJECT_VERSION_PATCH_DIGIT})
+set(CSHARP_VERSION_BUILD 0)
+
+if (CMAKE_SIZEOF_VOID_P EQUAL 8)


https://cmake.org/cmake/help/latest/variable/CMAKE_CL_64.html

Discouraged. Use CMAKE_SIZEOF_VOID_P instead.
Set to a true value when using a Microsoft Visual Studio cl compiler that targets a 64-bit architecture.

jmarrec · 2020-04-28T13:49:47Z

csharp/CMakeLists.txt

+  # TODO: there are probably adjustments to make to OpenStudio.csproj_dotnet.in
+  # The Release/Debug can be stripped probably
+  # Also probably don't need to use multiple `<TargetFrameworks>` which is the reason there's an extra netcoreapp3.0/ level
+  set(OPENSTUDIO_CSHARP_DLL "${CSHARP_LIBRARY_OUTPUT_DIRECTORY}/${CMAKE_BUILD_TYPE}/netcoreapp3.0/OpenStudio.dll")


not sure what to do there.

jmarrec · 2020-04-28T13:49:52Z

csharp/CMakeLists.txt

-  install(TARGETS openstudio_csharp DESTINATION CSharp/openstudio/ CONFIGURATIONS RELEASE COMPONENT "CSharpAPI")
+add_library(
+  openstudio_csharp
+  SHARED # Was "MODULE" on Windows before


Can't remember why I had to change that to "SHARED" instead of "MODULE", not 100% sure the impact it'll have.

jmarrec · 2020-04-28T13:55:31Z

src/utilities/core/csharp/LanguageSpecific.i

+// Ignore the ostream operator<<, for CSharp where it'll throw a multiple definition
+%ignore openstudio::operator<<;
+%ignore openstudio::model::operator<<;


This is for the CSharp_OpenStudio_LeftShift__SWIG_1 multiple definitions.

…name of the generated library file (in ProjectMacros) IT WORKS!

…udio_model_csharp

jmarrec · 2020-04-29T23:56:11Z

(py38)julien@OpenStudio.Tests (Dotnet_updated *=)$ dotnet test -v n
Build started 4/30/2020 1:53:32 AM.
[...]
         Build completed.


Test run for /home/julien/Software/Others/OpenStudio/csharp/examples/OpenStudio.Tests/bin/Debug/netcoreapp3.0/OpenStudio.Tests.dll(.NETCoreApp,Version=v3.0)
Microsoft (R) Test Execution Command Line Tool Version 16.3.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

A total of 1 test files matched the specified pattern.

[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.4.0 (64-bit .NET Core 3.0.1)
[xUnit.net 00:00:00.43]   Discovering: OpenStudio.Tests
[xUnit.net 00:00:00.46]   Discovered:  OpenStudio.Tests
[xUnit.net 00:00:00.46]   Starting:    OpenStudio.Tests
[xUnit.net 00:00:00.56]   Finished:    OpenStudio.Tests

√ OpenStudio.Tests.BCLTest1.MeasureTypeTest [34ms]
√ OpenStudio.Tests.BasicTests.BasicModel [34ms]

Test Run Successful.
Total tests: 2
     Passed: 2
 Total time: 1.0495 Seconds
     1>Done Building Project "/home/julien/Software/Others/OpenStudio/csharp/examples/OpenStudio.Tests/OpenStudio.Tests.csproj" (VSTest target(s)).

Build succeeded.
    0 Warning(s)
    0 Error(s)

kbenne · 2020-06-02T17:24:29Z

src/model/ModelObject.hpp

  std::string m_componentTypeName;
  std::string m_controlTypeName;
 };

+/** Simple class for ScheduleTypeRegistry key. \relates ModelObject */
+class MODEL_API ScheduleTypeKey {


OK I see you changed the type of ScheduleTypeKey from std::pair. But this is a breaking API change. I wouldn't complain except I also saw that the std::pair was even deliberately swigged. So we shouldn't take this change lightly. Is it the only way? If so then at least we should say something in release notes.

Probably not mandatory, but much cleaner, and it does fix a C# bindings issue with an improperly wrapped type. There are multiple occasions where std::pair<std::string, std::string> was defined to something that was clashing too if I recall correctly.

Like src/utilities/core/Containers.hpp:58:typedef std::pair<std::string,std::string> StringPair;>

Adding it to the release notes would make sense.

I doubt anyone uses ScheduleTypeKey much anyways, this stuff is meant to be used by the ScheduleRegistry internally, not messed with.

I agree on all of your points. Can you just add a comment in the release notes and then I'll just go ahead and merge? I think you will have to start a new file https://github.com/NREL/OpenStudio/tree/develop/developer/doc/ReleaseNotes for version 3.1.

https://github.com/NREL/OpenStudio/pull/3960/files#diff-7f9f2001055db45a596d70f0e33579ce

kbenne · 2020-06-02T17:26:00Z

src/utilities/core/Compare.cpp

-VersionString::VersionString(const std::string& version)
-  : m_str(version)
-{
+bool VersionString::parseVersionString(const std::string& version) {


Are these changes to VersionString all cosmetic formatting changes, or is there a functional change hidden in here?

kbenne · 2020-06-02T17:26:38Z

src/utilities/core/Compare.hpp

@@ -114,6 +114,10 @@ class UTILITIES_API VersionString {
 public:
  explicit VersionString(const std::string& version);

+  // Default constructor, assumes the current OpenStudio::openStudioLongVersion is passed


Ok I think here is the new functional thing. Makes sense.

MingboPeng · 2020-08-03T04:00:59Z

Hi @jmarrec, could you please share the C# sdk for Ubuntu? I am trying to refactor the Ironbug to make it dotnet core compatible, and run on Ubuntu. I cannot really figure out how to compile the OpenStudio C# SDK on Ubuntu. Could you please provide some insights?

Thanks,
Mingbo

jmarrec · 2020-08-03T05:48:25Z

@MingboPeng You should only need to install dotnet-sdk on ubuntu, and enable BUILD_CSHARP_BINDINGS=ON in CMake. I can walk you through it via slack (On UnmetHours' slack for eg) if you want.

MingboPeng · 2020-08-03T20:36:07Z

With @jmarrec 's help, I can confirm that C# bindings works on my Ubuntu as well. Nice work!

jmarrec · 2020-08-04T06:13:56Z

Great news and Congrats on being the first person to try it out :D

MingboPeng · 2020-10-20T03:34:44Z

Hi @jmarrec, I am starting to build a Github workflow (CI) to automatically build this CSharpSDK on Ubuntu. But I got a several issues:

EnergyPlus9.4 HASH mismatch

https://github.com/MingboPeng/OpenStudio/runs/1278780180?check_suite_focus=true#step:5:613

Issue with openstudio_ruby
https://github.com/MingboPeng/OpenStudio/runs/1278780180?check_suite_focus=true#step:5:833

Please let me know if I missed anything.
Thanks in advance.

jmarrec · 2020-10-21T07:02:56Z

@MingboPeng Use ubuntu-18.04 image, and you will not get the hash mismatch. I started adding the block for 20.04 for E+ since they make such an installer, but it was more for future proofing. Latest updates to E+ package didn't update the hash for the ubuntu20.04 installer.

20.04 isn't officially supported, we officially support 18.04!

For 2., it's number 1. that's causing the error. Just because CMake says it didn't configure properly after a ruby message doesn't mean it's from ruby. the CMake Error due to file download hash mistmatch is likely the root cause instead.

jmarrec · 2020-10-21T07:17:31Z

I've made some edits to your buildCsharp.yml github action file in a PR at MingboPeng#1

MingboPeng · 2020-10-22T04:00:43Z

Thanks @jmarrec, on ubuntu, I think all works now.

I am also trying to compile it on Mac, but got following error that I can't resolve.

It failed to find the compiler while the c compiler exists.
https://github.com/MingboPeng/OpenStudio/runs/1290457540?check_suite_focus=true#step:7:449

https://github.com/MingboPeng/OpenStudio/runs/1290457540?check_suite_focus=true#step:7:17

Thanks again for your time.
Mingbo

macumber and others added 30 commits July 13, 2018 11:07

Merge pull request #3176 from NREL/iteration

4cff071

Iteration

Merge remote-tracking branch 'remotes/origin/iteration'

9dff44e

Improve the runtime performance of ViewFactor

e5ff691

close #3713 (cherry picked from commit 09da50c)

Merge branch 'master' of https://github.com/NREL/OpenStudio into v2.9…

45858ab

….0-rc3

Merge pull request #3723 from NREL/v2.9.0-rc3

801faa4

Updating master for 2.9.0 release

Merge branch 'master' into v2.9.1-rc1

cba36e8

fix comment space

5b1bc68

Merge pull request #3803 from NREL/v2.9.1-rc1

e9e18ad

Updating to v2.9.1

Merge remote-tracking branch 'origin/develop' into v3.0.0-rc3

5837217

whitespace changes in bundle tests [chore]

850f74a

Fix #3311: use workflow gem instead of standards to skin down disk usage

d7e0bec

Avoid some C# annoying warnings about 'xxx' is a C# keyword

ba10eff

Avoid thousands of echo "cwstring.i not implemented for this target"

d6f24cc

cwstring.i is only implemented for python and tcl really.

Help in overload resolution to suppress C# warning 516

be5c801

Resolve some SWIGTYPE_p problems

a4ab9be

Try to avoid warning overloaded toString, not exactly working still.

126aa7d

SWIG std::pair<VersionString, std::string>. But VersionString needs a…

b3d9f7c

… default constructor, so make one

Merge branch 'v3.0.0-rc3'

b2bc2e8

Add a proper helper class for ScheduleTypeKey

dca3876

Swig isn't happy if you do two typedef/templates for the same stuff (std::pair<std::string, std::string> here)

Warnign treated as error on MSVC

bf1ba58

Try to use dotnet core on Linux

b128f7a

Expose BUIlD_CSHARP on something else than MSVC

7cdd37c

compile flags and stuff has to be inside if(MSVC)

6dc182c

Add no deprecated warnings (turned error) since we do export the depr…

7d350be

…ecated declarations to C#

CMAKE_CL_64 is discouraged and works only for msvc

b1fb30b

whitespace

6c2150c

modernize csproj for netcoreapp3.0

8b282d8

Try to simply ignore the ostream operator<< overloads for c# since th…

f534f52

…at causes multiple definitions

Try to allow building nuget package outside of MSVC

d10509c

jmarrec commented Apr 28, 2020

View reviewed changes

jmarrec self-assigned this Apr 28, 2020

jmarrec added component - C# Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Apr 28, 2020

Try to consume the local nuget pkg (not working)

ae255b1

jmarrec mentioned this pull request Apr 28, 2020

Dotnet updated jmarrec/OpenStudio#7

Closed

jmarrec added 6 commits April 29, 2020 22:43

After creating a MCVE, realized that the -dllimport should match the …

5099504

…name of the generated library file (in ProjectMacros) IT WORKS!

Can't manage to load the nuget nupkg, but locally this works!

81349e6

Try to upgrade OpeNStudio.targets, remove duplicate include of openst…

8b3582f

…udio_model_csharp

test a few more things in the csproj

e99d443

Create a C# unit test project that can be run via dotnet test

48889ad

Cleanup console output

f796c3e

Merge remote-tracking branch 'origin/develop' into Dotnet_updated

2d7e2fb

kbenne reviewed Jun 2, 2020

View reviewed changes

jmarrec added 2 commits June 2, 2020 21:08

Add release notes for #3959 and #3960 for 3.0.1

d6a6525

Actually add it to 3.1.0 release notes

28fa405

kbenne merged commit f339397 into develop Jun 2, 2020

kbenne deleted the Dotnet_updated branch June 2, 2020 19:19

jmarrec mentioned this pull request Aug 6, 2020

64/32 bit universal nuget package. #4027

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SWIG C# fixes + dotnet support #3960

SWIG C# fixes + dotnet support #3960

jmarrec commented Apr 28, 2020 •

edited

jmarrec left a comment

jmarrec Apr 28, 2020

jmarrec Apr 28, 2020

jmarrec Apr 28, 2020

jmarrec Apr 28, 2020

jmarrec Apr 28, 2020

jmarrec Apr 28, 2020

jmarrec commented Apr 29, 2020 •

edited

kbenne Jun 2, 2020

jmarrec Jun 2, 2020

kbenne Jun 2, 2020

jmarrec Jun 2, 2020

jmarrec Jun 2, 2020

kbenne Jun 2, 2020

kbenne Jun 2, 2020

MingboPeng commented Aug 3, 2020

jmarrec commented Aug 3, 2020

MingboPeng commented Aug 3, 2020

jmarrec commented Aug 4, 2020

MingboPeng commented Oct 20, 2020

jmarrec commented Oct 21, 2020 •

edited

jmarrec commented Oct 21, 2020

MingboPeng commented Oct 22, 2020

SWIG C# fixes + dotnet support #3960

SWIG C# fixes + dotnet support #3960

Conversation

jmarrec commented Apr 28, 2020 • edited

Pull request overview

Pull Request Author

Review Checklist

jmarrec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarrec commented Apr 29, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MingboPeng commented Aug 3, 2020

jmarrec commented Aug 3, 2020

MingboPeng commented Aug 3, 2020

jmarrec commented Aug 4, 2020

MingboPeng commented Oct 20, 2020

jmarrec commented Oct 21, 2020 • edited

jmarrec commented Oct 21, 2020

MingboPeng commented Oct 22, 2020

jmarrec commented Apr 28, 2020 •

edited

jmarrec commented Apr 29, 2020 •

edited

jmarrec commented Oct 21, 2020 •

edited