Skip to content
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

Managing CoolProp interface dependencies - Tree structure #8

Closed
slamer59 opened this issue Nov 22, 2017 · 9 comments
Closed

Managing CoolProp interface dependencies - Tree structure #8

slamer59 opened this issue Nov 22, 2017 · 9 comments

Comments

@slamer59
Copy link

The goal of ExternalMedia library is to provide a framework for interfacing external codes computing fluid properties to Modelica.Media-compatible component models.

The dependency will follow the recommendation stated by @casella and @adrpo : Download the sources and build the dependency

How to integrate CoolProp in ExternalMedia project:

  1. As a submodule in git
    I don't know how submodule works in git but many blogs seems to say that is a really difficult path to make it work simply. I might be wrong.

  2. git subtree
    Alternative to git submodules : git subtree

  3. External CMake project (ExternalProject_Add).
    This how to explain a simple integration: External CMake Project

For me, the third solution is easier to maintain and understand.
What do you think ? What do I miss ?

@casella
Copy link
Collaborator

casella commented Nov 22, 2017

I'm not much into this kind of technical details, but I would favour approach n. 3. Having CoolProp as a submodule or subtree of ExternalMedia is wrong from a logical point of view. It would be like having the United States as a province of Italy 😄

Furthermore, if in the future people want to add more solvers in a clean way, 3. seems much better than 1. and 2., otherwise we end up also adding China, India etc. as other provinces of Italy...

Ideally, it should be possible for anyone to choose which solvers should be actually compiled into the library, possibly including their private own one, and this seems to be easier to do with 3.. We will provide pre-compiled libraries with RefProp and CoolProp support, but that's by no means exhaustive of what you can do with this library.

@bilderbuchi
Copy link
Collaborator

I have no strong feelings in either direction (@casella's reasoning above seems sound, though). I only caution that many of the git submodule problems have stemmed from things since fixed in current-ish git versions, so I would de-emphasize the weight of old articles on the internet.

@slamer59
Copy link
Author

I update Build-CMake.sh

Compilation seems to work fine pointing to v6.1.0 tag of CoolProp

@jowr
Copy link
Collaborator

jowr commented Dec 21, 2020

I may have some time next year - I am going to fix this by including External CMake project using ExternalProject_Add and an option to disable CoolProp.

Please let me know if working on this is obsolete: @slamer59 and @casella

@slamer59
Copy link
Author

Hello,
I move to others initiatives. For me coolprop is still what is lacking inside open source modelica. It has been hard to compiled it and difficult to maintain and each time someone asked for help nobodies was there unfortunately (me included).

So... I think it worth it to make it clean and good documentation on how it works and focus on OSS.

@jowr
Copy link
Collaborator

jowr commented Dec 21, 2020

Fine I'll have a look at it.

I may reach out at some point and ask for a review of my changes - I hope that is OK.

@jowr jowr mentioned this issue Dec 22, 2020
@casella
Copy link
Collaborator

casella commented Dec 22, 2020

I may have some time next year - I am going to fix this by including External CMake project using ExternalProject_Add and an option to disable CoolProp.

Please let me know if working on this is obsolete: @slamer59 and @casella

I've been playing around with the idea of using the pre-compiled CoolProp dll, instead of compiling it from source. This makes the project management a lot easier. I made some proof-of-concept experiments and it seems to work fine.

@jowr, I should have time to work on this during or immediately after the Christmas break, maybe on a branch. What do you think?

@casella
Copy link
Collaborator

casella commented Dec 22, 2020

See discussion here

@beutlich beutlich linked a pull request Dec 22, 2020 that will close this issue
This was referenced Jan 4, 2021
@jowr
Copy link
Collaborator

jowr commented May 5, 2021

This has been foxed for v3.3.0

@jowr jowr closed this as completed May 5, 2021
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 a pull request may close this issue.

4 participants