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

add fugacity funcs compatible with Sept 2023 CoolProp master branch #33

Closed

Conversation

chris-hampel-CA
Copy link
Contributor

Closes #31

Fugacity lookups did not exist in CoolProp.jl. They are added here. However, a fugacity function in the base C++ which is compatible with Julia wrapping did not exist previously. The C++ code was added successfully. CoolProp/CoolProp#2286

This will be a draft PR until CoolProp releases a new version that includes the change which is ahead of v6.5. Once that is released and CoolProp_jll is updated, this PR can be merged.

@barche
Copy link
Collaborator

barche commented Sep 13, 2023

Do you have any idea on the timeline for the CoolProp release? We could also release a JLL based on the git version.

@ibell
Copy link
Contributor

ibell commented Sep 13, 2023

No, but I would like to get as many of the low-level passthrough functions implemented at the same time to avoid making a million little releases since it takes a while to do each release

@chris-hampel-CA
Copy link
Contributor Author

chris-hampel-CA commented Sep 13, 2023

I have been working on a comprehensive review of functions existing in AbstractState.h here
and cross-referencing them against what exists in CoolPropLib.h and what can be output from a PT_INPUTS call. This is in effort to figure out the extent of the proposed work to as many low-level passthrough functions implemented before releasing another version. Here is a brief summary of what I have found so far.

These functions in AbstractState.h cannot be called with a PT_INPUTS call and do not have a passthrough function currently.

For property lookups:

  1. all properties with the _residual tag
  2. all properties with the _excess tag
  3. fugacity_coefficients with an s on the end (only added fugacity and fugacity_coefficient passthrough functions so far)
  4. chemical_potential
  5. several alpha derivative terms (I have the full list)
  6. second_partial_deriv

Several other miscellaneous functions (I do not know which are critical to include as passthrough functions):
8. get_mass_fractions
9. isSuperCriticalPhase
10. calc_conductivity_contributions
11. the list goes on for 10-20 or so more functions

@barche
Copy link
Collaborator

barche commented Sep 14, 2023

OK, I'll look at merging the backlog of PRs here, first also accepting the upgrade to CoolProp 6.5. Note that we could also switch to using CxxWrap to use the C++ interface directly, that might be an easier solution?

@ibell
Copy link
Contributor

ibell commented Sep 14, 2023

I have been working on a comprehensive review of functions existing in AbstractState.h here and cross-referencing them against what exists in CoolPropLib.h and what can be output from a PT_INPUTS call. This is in effort to figure out the extent of the proposed work to as many low-level passthrough functions implemented before releasing another version. Here is a brief summary of what I have found so far.

These functions in AbstractState.h cannot be called with a PT_INPUTS call and do not have a passthrough function currently.

For property lookups:

  1. all properties with the _residual tag
  2. all properties with the _excess tag
  3. fugacity_coefficients with an s on the end (only added fugacity and fugacity_coefficient passthrough functions so far)
  4. chemical_potential
  5. several alpha derivative terms (I have the full list)
  6. second_partial_deriv

Several other miscellaneous functions (I do not know which are critical to include as passthrough functions): 8. get_mass_fractions 9. isSuperCriticalPhase 10. calc_conductivity_contributions 11. the list goes on for 10-20 or so more functions

If you want to implement them all, that would be fine, but I personally think it makes sense to wait for someone to want it rather than implementing it before there is a need. But if it comes to the Julia wrapper, it might make sense to just do all the interfacing in one shot.

@chris-hampel-CA
Copy link
Contributor Author

I agree that it makes more sense to add them as they come up, but then that could lead to many PR's and release requests as you fear. However, it is another good chunk of work to do one big push, that no one else is requesting right now. Really, we only need these fugacity funcs in a C++ release to be able to stably deploy the use of CoolProp.jl for our internal modeling applications at our organization. Having to locally dev a CoolProp.jl which devs a local CoolProp_jll is cumbersome to work with in an organization of many users, in the end. However, I understand the world does not revolve around our immediate needs.

We would be willing to make at least "some more" of these passthrough functions right now and maybe in exchange we could get a C++ release done. Perhaps, adding remaining property lookups, like fugacity, make the most sense and could be a good compromise. Let me know your thoughts. Thanks

@n-aswin
Copy link

n-aswin commented Sep 16, 2023

If it helps, this fork has some of the low-level functions added in branch update-CoolProp-library. The branch update-Julia-wrapper has the same added to the Julia wrapper. Not fully tested though.

I had planned to submit the changes after adding all of the low-level functions and attempting to update the Julia wrapper (at least for local use if not merged upstream) with following changes:

Progress on the above is in new-Julia-wrapper branch.

@chris-hampel-CA
Copy link
Contributor Author

Thanks for the info @n-aswin. I will review what you have shared and will see where I might be able to help.

In the meantime, since I've only needed the fugacity calls, specifically, for my current work, I was able to sleuth how to make a local version of the CoolProp_jll pkg based on the new additions to CoolProp that have not been released yet. The results show that the CoolProp.jl changes proposed in this PR will work once a CoolProp release is made.

image

@barche
Copy link
Collaborator

barche commented Sep 24, 2023

Just to be clear: we wait with merging this one until a new CoolProp comes out, right?

@chris-hampel-CA
Copy link
Contributor Author

Just to be clear: we wait with merging this one until a new CoolProp comes out, right?

That is correct.

@longemen3000
Copy link
Contributor

@barche CoolProp 6.6.0 has been released and added to Yggdrasil

@barche barche mentioned this pull request Dec 6, 2023
@barche
Copy link
Collaborator

barche commented Dec 6, 2023

I have added an updated version (rebased on master) at PR #35, let's continue the discussion there.

@barche barche closed this Dec 6, 2023
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.

Unable to return fugacity lookups using the julia wrapper
5 participants