Testing CoolProp 6 (6.4.2) #27

dongkeun-oh · 2021-03-25T11:08:55Z

Hi, this is Dong Keun with KFE (Korean Institute of Fusion Energy, former NFRI) in Korea.

It's a pilot to check the interoperability of the newest CoolProp (v 6.4.2).
As I left a comment on the issue #24, there are 3 items to solve with the latest CoolProp.

Exception handling should be careful against the ValueError(s), in particular, in setState_ph function during update, which makes a troubles with an uncontrolled exiting in the external library call.
DLL should be installed to exploit the new feature of NF to initialize constants, which is essential to make fiudConstants work.
REFPROP doesn't work yet.

By means of this test, item 1 and 2 seem to be solved; the most radical attempt in this test is "building DLL (shared object) instead of static library". So, I have to ask you whether it's fine to be accepted; I just checked that the DLL is much faster than the static library in compilation. Moreover, I would recommend not to pull this commit to the master branch, because it's just a test to share my attempt discussing whether this approach is reasonable. Then, your feedback is very appreciated for me to go ahead.

I tested the following model, which doesn't work without my amendment, and it works fine in Windows 10 (64bit MSYS, gcc 10.2.0) as well as in Linux (Ubuntu 18.04, gcc 7.5.0).

within ExternalMedia;
model TestCoolProp
  package CO2 extends ExternalMedia.Media.CoolPropMedium(
      mediumName = "CarbonDioxide",
      substanceNames = {"CO2|debug=0"},
      ThermoStates = Modelica.Media.Interfaces.Choices.IndependentVariables.ph,
      SpecificEnthalpy(start=hstart));
  end CO2;
  parameter CO2.SpecificEnthalpy hstart = 1e5;
  CO2.BaseProperties baseProperties(h(start=hstart));
  CO2.AbsolutePressure p = 1e6;
  CO2.Temperature T = 280.0 + 50*time;
equation
  baseProperties.p = p;
  baseProperties.T = T;  
end TestCoolProp;

Another important thing is the recent CP 6.4.2 gives error configuring the object library; I issued a pull request to Ian, and you can solve immediately by removing the line of target_link_libraries which makes such an error.

Later, I will be pleased to activate a discussion to improve how to handle the cases of "out of range" in the runtime process, and I hope to contribute more to the issue of REFPROP.

…A_ERRORS=1

casella · 2021-03-29T22:26:29Z

@dongkeun-oh, thank you very much for your contribution.

I'm try to get again up to speed with this project after a long delay, it's a bit hard for me to keep up among multiple tasks, sorry about that.

I agreed with @jowr that we should first stabilize version 3.3.0 with CoolProp 5, and possibly release it, then we can move on to CoolProp 6. He already made PR #26 with many improvements, that I am going to test and merge in ASAP. We could then try what you put in this PR.

Is that ok for you?

Thanks!

dongkeun-oh · 2021-03-30T02:06:57Z

Sure!

First of all, would you check 'TestCoolProp' which I attached to my previous message? As I figured out, the CoolProp 5 has the same problems which I listed up in my original comment #24.
Actually, it is an attempt just to check up such a problem; I need to update my PR after your tests, because I touched setState_ph only for this test, and I will do the same thing for the other routines based on our discussion.
So, let me ready to go ahead on your comment.

Thanks a lot, too!

jfulem · 2021-04-15T20:30:12Z

I tested the CoolProp/CoolProp#2016, but I'm getting some issue:

forrtl: severe (59): list-directed I/O syntax error, unit 12, file c:\Program Files\Refprop\fluids\WATER.fld
Image              PC                Routine            Line        Source
REFPRP64.dll       00000001801DCB47  Unknown               Unknown  Unknown
REFPRP64.dll       00000001801D8353  Unknown               Unknown  Unknown
REFPRP64.dll       00000001801A1066  Unknown               Unknown  Unknown
REFPRP64.dll       0000000180186BE5  Unknown               Unknown  Unknown
REFPRP64.dll       0000000180185820  Unknown               Unknown  Unknown
REFPRP64.dll       0000000180161ED9  Unknown               Unknown  Unknown
REFPRP64.dll       000000018016022C  Unknown               Unknown  Unknown
REFPRP64.dll       000000018003B154  Unknown               Unknown  Unknown

I'm using Refprop 10.

Do you know why this might happen?

jowr · 2021-04-16T08:39:32Z

I agree with @casella - there are a lot of good ideas here, but my focus is to release something and then look at updating CoolProp. Also moving to a DLL has been a long term goal, thanks for bringing it up again.

…ICA=OFF in default

dongkeun-oh · 2021-05-04T12:30:08Z

Hi, friends~~
I merged v.3.3.0-dev. which recently pulled the PR #34 of @jowr (#), and worked out for the CoolProp 6 and the OMC's NF based on the merge.
So now,

A new option OPENMODELICA (OFF in default) is defined to activate the DLL target for Win64 (MinGW64) and Linux64; the DLL is required for the OMC's NF.
To generate the DLL, I work around the SimulationRuntimeC and omcgc by setting EXTERNALMEDIA_MODELICA_ERRORS=0 , which is the previous manner of @jowr.
In Linux, the function errorMessage() makes the model (TestBasePropertiesImplicit) hang; windows is fine, ridiculously. So, I employed warningMessage() instead.

To test this PR in OpenModelica, please just try

cmake .. -DOPENMODELICA=ON in linux,
cmake .. -G "MSYS Makefiles" -DOPENMODELICA=ON -DFLUIDPROP=ON in windows (mingw64)

I found MSVC (VS2019) doesn't work yet..

Do not keep old junk as comments

jowr

Thank you for your updates.

Could you please explain why you create an additional target *_DLL? For me this should be an option - either one wants the shared or the static library, but creating both probably only creates confusion.

I am going to include some of our suggestions in v3.3.0 and then we can focus on OpenModelica and CoolProp v6.4.2 in v3.4.0.

jowr · 2021-05-04T12:47:28Z

Projects/CMakeLists.txt

@@ -49,7 +49,11 @@ option(FLUIDPROP "Include the FluidProp library for fluid properties" OFF)
 if(COOLPROP)
  # Get the CoolProp code
  if(NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git")
-    file(DOWNLOAD https://sourceforge.net/projects/coolprop/files/CoolProp/5.1.1/source/CoolProp_sources.zip/download "${CMAKE_BINARY_DIR}/coolprop.zip")
+    if(NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/../externals")
+		file(MAKE_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/../externals")


This is redundant, see line 37

jowr · 2021-05-04T12:48:02Z

Projects/CMakeLists.txt

@@ -41,6 +40,7 @@ endif()

 option(COOLPROP "Include the CoolProp library for fluid properties" ON)
 option(FLUIDPROP "Include the FluidProp library for fluid properties" OFF)
+option(OPENMODELICA "Add the shared library required by the OpenModelica's NF" OFF)


Why would you introduce an extra option? Can't we just use the shared library switch for this?

Please see the main part of comment below..

jowr · 2021-05-04T12:50:22Z

Projects/CMakeLists.txt

@@ -60,26 +64,31 @@ if(COOLPROP)
    #file(ARCHIVE_EXTRACT INPUT ${CMAKE_BINARY_DIR}/coolprop.zip)
    #execute_process(COMMAND unzip ${CMAKE_BINARY_DIR}/coolprop.zip -d "${CMAKE_BINARY_DIR}/coolprop-tmp")
    #execute_process(COMMAND sed -i '421s;^;\#;' "${CMAKE_BINARY_DIR}/coolprop-tmp/CoolProp.sources/CMakeLists.txt")
-    file(MAKE_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/../externals")
-    file(RENAME "${CMAKE_BINARY_DIR}/coolprop-tmp/CoolProp.sources" "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git")
+	file(RENAME "${CMAKE_BINARY_DIR}/coolprop-tmp/CoolProp.sources" "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git")


Be careful with the indentation 😉 - no change needed here

jowr · 2021-05-04T12:52:30Z

Projects/CMakeLists.txt

  # Add the CoolProp sources
  ADD_SUBDIRECTORY ("${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git" "${CMAKE_CURRENT_BINARY_DIR}/CoolProp")
  list (APPEND INCLUDE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git")
  list (APPEND INCLUDE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git/include")
  list (APPEND INCLUDE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git/externals/msgpack-c/include")
  list (APPEND INCLUDE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git/externals/cppformat")
+  list (APPEND INCLUDE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git/externals/fmtlib")
+else()
+  list(REMOVE_ITEM LIB_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/Sources/coolpropsolver.cpp")


This should not be required - please note that C++ macros handle this.

Do you mean 90 an 91?

jowr · 2021-05-04T12:53:43Z

Projects/CMakeLists.txt

@@ -134,12 +143,12 @@ set_property(TARGET ${LIBRARY_NAME} PROPERTY VERSION ${APP_VERSION})
 target_compile_definitions(${LIBRARY_NAME} PRIVATE EXTERNALMEDIA_FLUIDPROP=$<IF:$<BOOL:${FLUIDPROP}>,1,0>)
 target_compile_definitions(${LIBRARY_NAME} PRIVATE EXTERNALMEDIA_COOLPROP=$<IF:$<BOOL:${COOLPROP}>,1,0>)
 target_compile_definitions(${LIBRARY_NAME} PRIVATE EXTERNALMEDIA_MODELICA_ERRORS=1) # Use 0 for a shared library and 1 for a static library
-#target_compile_definitions(${LIBRARY_NAME} PRIVATE EXTERNALMEDIA_LIBRARY_EXPORTS) # Use this for a shared library
+


Why did you remove this? I think it actually is required for Windows DLLs

Thank you for your updates.

Could you please explain why you create an additional target *_DLL? For me this should be an option - either one wants the shared or the static library, but creating both probably only creates confusion.

I am going to include some of our suggestions in v3.3.0 and then we can focus on OpenModelica and CoolProp v6.4.2 in v3.4.0.

In the recent update in openmodelica, the new-frontend (NF) just requires such a DLL to initialize any constant by an external function. So, I just took a conservative manner to preserve the original cmake script as much as possible, and to add up the DLL only in case of openmodelica; if you like the DLL for a major option of the build process, we can define such an option to generate the DLL instead of the static library. Do you prefer that manner?

jowr · 2021-05-04T18:33:01Z

This is super valuable - OpenModelica-support has been a long-term goal. I think we should take it in two steps

Upgrade CoolProp
Fix OpenModelica and compile to DLL.

What do you think?

dongkeun-oh · 2021-05-06T03:14:10Z

This is super valuable - OpenModelica-support has been a long-term goal. I think we should take it in two steps
1. Upgrade CoolProp

2. Fix OpenModelica and compile to DLL.
What do you think?

Thank you for the comment!!
It just makes sense to discuss such a plan in detail around here.

First for all, I have tested with two different nightly-built (1.18.0~dev) set-ups - one is in Ununtu 18.04 (nightly-built binary package with gcc 7.5), and the other is Windows 10 (source build by clang++ from GitHub with gcc 10.2)
So now,

The long lasting trouble with OM due to fluidConstant was solved by the effort using DLL; see https://trac.openmodelica.org/OpenModelica/ticket/5240 and https://trac.openmodelica.org/OpenModelica/ticket/6331. That's why I'm using such a recent version.
After all, it works with the latest CoolProp 6.4.2-dev; all the tests in ExternalMedia.Tests.CoolProp.CO2 are passed without a trouble.
Surely, DLL helps because compilation is much faster than static library.

Then, there are issues, potentially, under discussion.

I guess it's recommendable to have a regular option of DLL target instead of static library, discarding my patch of additional target, and an idea on the Modelica message, which looks possible in OM by linking SimulationRuntimeC and omcgc without any idea for Dymola, to generate the DLL. So, an option in case of OM can be thought to select those libraries for DLL target.
With CoolProp, taking ValueError seems essential to identify simple errors in the output, avoiding the troubles of hang, for instance, due to the results out of range. So, it looks necessary to review the CoolProp sources in such a point of view.
Also, REFPROP does not work yet; I hope it will be done soon, but, not immediately (A commit for the REFPROP backend to include the functions of satu… CoolProp/CoolProp#2016).
My actual target is integrating this media library to ThermoPower components. After stabilization by discussion with you, I would move to activate that.

How about you?

casella · 2021-06-20T10:01:30Z

Folks, I'm sorry these months have been terrible for me. I hope I will be able to pick this up from next week

bilderbuchi · 2021-09-21T11:57:04Z

@dongkeun-oh have there been any updates on this PR in the meantime? are you able to fix the merge conflict that is present in the PR?

bilderbuchi · 2021-09-23T12:53:29Z

Also, when the conflict is removed and CI compiles artifacts, I can offer to test on Windows 10 with OpenModelica 1.18. if desired. I'm less sure about compiling everything myself.

dongkeun-oh · 2021-09-23T13:54:05Z

I'll be back to this issue in the middle of October or later.
Sorry for the delay.

mww1990 · 2021-09-23T13:55:22Z

Also, when the conflict is removed and CI compiles artifacts, I can offer to test on Windows 10 with OpenModelica 1.18. if desired. I'm less sure about compiling everything myself.

Hello,
feel free to use one of the pre-compiled packages (Coolprop Nightly Version and Coolprop 6.4.1) including a *.dll for shared library:
Coolprop 6.4.1:
https://cloud.tugraz.at/index.php/s/Sx27BG6gxQDgbiC
or
Coolprop Nightly:
https://cloud.tugraz.at/index.php/s/DdK6wyDC89fCg5a

I tested both and currently use them e.g. for the calculation of ammonia properties in various circuits on a daily basis. You might try BICUBIC and TTSE interpolation for the calculation of fluid properties. (i found that this might cause some errors using the Coolprop Nightly Version)

bilderbuchi · 2021-09-24T06:58:46Z

Thank you Michael, much appreciated! Did you have to do any patching/tweaking on ExternalMedia to achieve that? If so, could that be upstreamed here? E.g. creating both static and dynamic libraries in the CI/releases could be useful I reckon.

bilderbuchi · 2022-02-16T06:14:50Z

@jowr did you close this PR intentionally, or was that a side-effect of deleting the target branch?

jowr · 2022-02-16T16:32:44Z

That was not intended. However, #49 updates CoolProp to v6.4.1 which my render this PR obsolete. However, we still have to fix the DLL compilation, but that is mostly addressed by @casella ...

dongkeun-oh and others added 3 commits March 25, 2021 15:11

Suppressing the trouble of "out of range", mainly, by setting MODELIC…

e4157f4

…A_ERRORS=1

An amendment just for test with 64bit MinGW in Windows 10

ff9856f

Fxing a bug in Linux after the last amendment..

156f981

dongkeun-oh mentioned this pull request Mar 29, 2021

Missing member functions in the REFPROP backends (calc_first_saturation_deriv etc) CoolProp/CoolProp#2013

Open

casella mentioned this pull request Mar 30, 2021

v.3.3.0 with CI and documentation #26

Merged

4 tasks

dongkeun-oh mentioned this pull request Mar 31, 2021

Some tests fail at runtime with v.3.3.0 due to wrong error management #28

Closed

dongkeun-oh mentioned this pull request Apr 13, 2021

A commit for the REFPROP backend to include the functions of satu… CoolProp/CoolProp#2016

Open

dongkeun-oh and others added 4 commits May 3, 2021 18:54

Merge branch 'v.3.3.0-dev' into testCP6

c8d8748

Testing new merge from the update of v.3.3.0-dev

0423901

Finalizing update after merge with v.3.3.0-dev

c80bd8f

Update for linux: suppressing hang by "errorMessage" + Make OPENMODLE…

2ce8bbe

…ICA=OFF in default

Update errorhandling.cpp

e71bae0

Do not keep old junk as comments

jowr reviewed May 4, 2021

View reviewed changes

jowr mentioned this pull request May 5, 2021

Update CoolProp #37

Closed

egroegphysics mentioned this pull request Jul 23, 2021

Build Tools Versioning #41

Closed

This was referenced Aug 4, 2021

WaterComparison does not compile with OM 1.18 beta1 #42

Closed

getting started #40

Closed

jowr deleted the branch modelica-3rdparty:v.3.3.0-dev February 15, 2022 15:25

jowr closed this Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Testing CoolProp 6 (6.4.2) #27

Testing CoolProp 6 (6.4.2) #27

dongkeun-oh commented Mar 25, 2021 •

edited

Loading

casella commented Mar 29, 2021

dongkeun-oh commented Mar 30, 2021

jfulem commented Apr 15, 2021

jowr commented Apr 16, 2021

dongkeun-oh commented May 4, 2021

jowr left a comment

jowr May 4, 2021

dongkeun-oh May 4, 2021

jowr May 4, 2021

dongkeun-oh May 4, 2021

jowr May 4, 2021

dongkeun-oh May 4, 2021

jowr May 4, 2021

dongkeun-oh May 4, 2021

jowr May 4, 2021

dongkeun-oh May 4, 2021

jowr commented May 4, 2021

dongkeun-oh commented May 6, 2021

casella commented Jun 20, 2021

bilderbuchi commented Sep 21, 2021

bilderbuchi commented Sep 23, 2021

dongkeun-oh commented Sep 23, 2021

mww1990 commented Sep 23, 2021 •

edited

Loading

bilderbuchi commented Sep 24, 2021

bilderbuchi commented Feb 16, 2022

jowr commented Feb 16, 2022

Testing CoolProp 6 (6.4.2) #27

Testing CoolProp 6 (6.4.2) #27

Conversation

dongkeun-oh commented Mar 25, 2021 • edited Loading

casella commented Mar 29, 2021

dongkeun-oh commented Mar 30, 2021

jfulem commented Apr 15, 2021

jowr commented Apr 16, 2021

dongkeun-oh commented May 4, 2021

jowr 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

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

jowr commented May 4, 2021

dongkeun-oh commented May 6, 2021

casella commented Jun 20, 2021

bilderbuchi commented Sep 21, 2021

bilderbuchi commented Sep 23, 2021

dongkeun-oh commented Sep 23, 2021

mww1990 commented Sep 23, 2021 • edited Loading

bilderbuchi commented Sep 24, 2021

bilderbuchi commented Feb 16, 2022

jowr commented Feb 16, 2022

dongkeun-oh commented Mar 25, 2021 •

edited

Loading

mww1990 commented Sep 23, 2021 •

edited

Loading