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

New functions for CoolPropLib #2133

Merged
merged 1 commit into from May 18, 2022
Merged

Conversation

friederikeboehm
Copy link
Contributor

Props1SImulti new function in CoolProp and Props1SImulti and PropsSImulti to CoolPropLib

Description of the Change

Function Props1SImulti was added to CoolProp. It is the Props1SI function but with multiple outputs possible, comparable to PropsSImulti.
Props1SImulti and PropsSImulti were added to CoolPropLib

Benefits

Enables the user to acces multiple non-thermodynamic-state-dependend fluid properties at once.

Possible Drawbacks

None

Verification Process

Functions were tested using the Mathematica wrapper, see PR #2085

@ibell
Copy link
Contributor

ibell commented May 9, 2022

What is the relationship between this PR and #2135 ? Should this be merged or closed?

@friederikeboehm
Copy link
Contributor Author

Both should be merged. I wanted to separate high-level and low-level functions (although I just realised #2135 has a high-level function as well...). Can join both PRs into one, if preferred.

@ibell
Copy link
Contributor

ibell commented May 10, 2022

My preference is to separate the low-level and high-level changes (PR should be quite granular, and do one thing only, within reason). I will merge this one.

@ibell
Copy link
Contributor

ibell commented May 10, 2022

Glad I thought about this, please refactor to move implementation into src/CoolProp.cpp

@friederikeboehm
Copy link
Contributor Author

Glad I thought about this, please refactor to move implementation into src/CoolProp.cpp

I'm sorry, I don't understand, what should I refactor? Props1SImulti is implemented in src/CoolProp.cpp

Props1SImulti new function in CoolProp
@ibell
Copy link
Contributor

ibell commented May 13, 2022

The function should be implemented in src/CoolProp.cpp, with STL types std::vector, etc., and the function in CoolPropLib.cpp should only do C->C++ datatype conversions and pass to the function in src/CoolProp.cpp, like all the other functions that are in src/CoolPropLib.cpp

throw CoolProp::ValueError(
format("Length of fractions vector [%d] is not equal to length of fluidNames vector [%d]", _fluidNames.size(), length_fractions));
std::vector<double> _fractions(fractions, fractions + length_fractions);
std::vector<std::vector<double>> _result = CoolProp::Props1SImulti(_outputs, backend, _fluidNames, _fractions);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call to CoolProp::Props1SImulti

@friederikeboehm
Copy link
Contributor Author

friederikeboehm commented May 13, 2022

The function should be implemented in src/CoolProp.cpp, with STL types std::vector, etc., and the function in CoolPropLib.cpp should only do C->C++ datatype conversions and pass to the function in src/CoolProp.cpp, like all the other functions that are in src/CoolPropLib.cpp

I thought that's exactly what I did? In CoolProp.cpp I implemented Props1SImulti and in CoolPropLib.cpp I call that function (and do a lot of error handling). I added comments at the according code changes in CoolProp.cpp and CoolPropLib.cpp. Please tell me, what I need to change as I don't understand what's wrong with my code.

@ibell ibell merged commit 44325d9 into CoolProp:master May 18, 2022
@ibell
Copy link
Contributor

ibell commented May 18, 2022

Sorry, I didn't read closely enough before

@friederikeboehm friederikeboehm deleted the props1SImulti branch May 18, 2022 06:34
zmeri pushed a commit to zmeri/CoolProp that referenced this pull request Aug 26, 2022
@jowr jowr added this to the v6.4.2 milestone Dec 7, 2022
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 this pull request may close these issues.

None yet

3 participants