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

PH_property bugs #320

Open
Sush1090 opened this issue Dec 18, 2024 · 9 comments
Open

PH_property bugs #320

Sush1090 opened this issue Dec 18, 2024 · 9 comments

Comments

@Sush1090
Copy link
Contributor

Hello,
I have two points:

  1. The following will fail in the new release,
julia> model = cPR(["ethane","methane"],idealmodel = ReidIdeal);

julia> p = 101325; T = 350;z = [1.2,1.2]; h = enthalpy(model,p,T,z)
5709.317234526068

julia> import Clapeyron.PH

julia> PH.entropy(model,p,h,z)

Error:

ERROR: UndefVarError: `T` not defined in local scope
Suggestion: check for an assignment to a local variable that shadows a global of the same name.

fix:
In PH_property(...) compute Tproperty outside phase if condition

T,calc_phase = _Tproperty(model,p,h,z,T0 = T0,phase = phase,threaded = threaded)
  1. Tproperty and Pproperty : I think if the input pressure or temperature given to the solver is above critical value the root finder can be initialized to critical value. I had mentioned it for Tproperty here but it can also be extended to Pproperty.

Thank you

@longemen3000
Copy link
Member

Uff, big bug, let me fix it and release a new version, if you find anything else let me know

@longemen3000
Copy link
Member

i saw the offending code, and the correct answer seems to remove the

        if np > 1
            return f(model,res)
        else
            return f(model,p,T,z)
        end

if condition and replace it with just f(model,res). the result of ph_flash has already an implicit calculation of Tproperty already
on 2. , we are already using the critical point as an initial point if the input is supecritical (at least in single phase):

if T >= Tc
verbose && @info "temperature is above critical temperature"
return __Pproperty(model,T,prop,z,property,rootsolver,phase,abstol,reltol,threaded,Pc)
end

if p >= Pc
verbose && @info "pressure is above critical pressure"
return __Tproperty(model,p,prop,z,property,rootsolver,phase,abstol,reltol,threaded,Tc)
end

@Sush1090
Copy link
Contributor Author

Hello,
There is a similar case for pure cases:

model = cPR(["ethane"],idealmodel = ReidIdeal);
p = 101325;h = 100; z = [1]; T = Clapeyron.PH.temperature(model,p,h,z)

error:

ERROR: UndefVarError: `T` not defined in local scope
Suggestion: check for an assignment to a local variable that shadows a global of the same name.
Stacktrace:
 [1] px_flash_pure(model::PR{ReidIdeal, TwuAlpha, NoTranslation, vdW1fRule}, p::Int64, x::Int64, z::Vector{Int64}, spec::typeof(enthalpy), T0::Nothing) 

longemen3000 added a commit that referenced this issue Dec 20, 2024
@longemen3000
Copy link
Member

longemen3000 commented Dec 20, 2024

good catch, just fixed. If everything goes alright, I'm releasing after tests pass

@Sush1090
Copy link
Contributor Author

There is also one in ps_flash
The following will not work:

model = cPR(["ethane"],idealmodel = ReidIdeal);
s = 100;p=101325;z = [1.0];
Clapeyron.PS.enthalpy(model,p,s,z)
ERROR: UndefVarError: `n` not defined in `Clapeyron`
Suggestion: check for spelling errors or missing imports.
Stacktrace:
 [1] ps_flash(model::PR{ReidIdeal, TwuAlpha, NoTranslation, vdW1fRule}, p::Int64, s::Int64, z::Vector{Float64}, method::GeneralizedXYFlash{Nothing, Nothing})

longemen3000 added a commit that referenced this issue Dec 20, 2024
@Sush1090
Copy link
Contributor Author

There is another one here:

model = cPR(["ethane"],idealmodel = ReidIdeal)
 p = 101325; z = [5.0]; 
 T = saturation_temperature(model,p)[1]
 h_liq = enthalpy(model,p,T-0.1,z);h_gas = enthalpy(model,p,T+0.1,z)
 h = (h_liq + h_gas)/2 
 PH.temperature(model,p,h,z)

error:

ERROR: LoadError: UndefVarError: `build_flash_result_pure` not defined in `Clapeyron`
Suggestion: check for spelling errors or missing imports.

longemen3000 added a commit that referenced this issue Dec 22, 2024
@Sush1090
Copy link
Contributor Author

Hello,
This is a different type of issue:

  1. The computation of pressure does not match
using Clapeyron 
using Clapeyron: PH
model = cPR(["ethane"],idealmodel = ReidIdeal)
 p = 101325; z = [5.0]; 
 T = saturation_temperature(model,p)[1]
 h_liq = enthalpy(model,p,T-0.1,z);h_gas = enthalpy(model,p,T+0.1,z)
 h = (h_liq + h_gas)/2 
 V = PH.volume(model,p,h,z)
 VT_p = pressure(model,V,T,z)
 @show p  VT_p
  1. This is give now LoadError I am using the latest branch. I think this was working before
using Clapeyron 
using Clapeyron: PH
model = cPR(["methane"],idealmodel = ReidIdeal)
p = 101325; z = [5.0]; 
T = 210.0 
h = enthalpy(model,p,T,z) 
V = PH.volume(model,p,h,z)
ERROR: MethodError: no method matching one(::StaticArraysCore.SVector{1, Float64})
The function `one` exists, but no method is defined for this combination of argument types.

The one that was fixed fails now this one

@longemen3000
Copy link
Member

on 1. that is because the result is in a two phase state. the volume in a two phase state is the sum of the volumes in each phase. i cannot reproduce 2. (and all the reported problems here run in CI).

btw i just added a flash function inside the PH, PS,PT and VT modules, maybe this is more useful for those cases

@Sush1090
Copy link
Contributor Author

the second one works now. I don't know what was wrong before

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

No branches or pull requests

2 participants