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.

Sign up for GitHub

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

Jump to bottom

Inconsistency in gas valve Cv calculation #1258

Closed
andr1976 opened this issue Jan 17, 2025 · 3 comments · Fixed by #1259
Closed

Inconsistency in gas valve Cv calculation #1258

andr1976 opened this issue Jan 17, 2025 · 3 comments · Fixed by #1259

Comments

@andr1976
Copy link

Describe the bug

I belive the code in ThrottlingValve adapts IEC / ANSI / ISA valve sizing for liquid service, but the Universal Gas Sizing for gas service, and this is a little confusing since the Cv in UGS is actually Cg , where Cg = Cv * Cl if I read the formulas right.

Further the sine factor used is for degrees, I suspect it should be radian i.e. Math.sin(59.64/Cl) (where Cl is assumed = 30). I am not 100% sure, since I dont code in java.
So, important information is that the Cv for gas valve is actually Cg, and Cv is Cg/30. Ideally both both liquid and gas would be on "Cv" basis. For gas could be fixed by coverting Cv to Cg when Cv is given as input and converting Cg to Cv when Cv is calculated.

For the sine function it appears in calcCv, calcPercentValveOpening, calcValvePout, calcmassflow

From: https://www.w3schools.com/java/ref_math_sin.asp
Definition and Usage
The sin() method returns the sine of an angle.

Note: Angles are measured in radians.

Tip: You can use the constant Math.PI to make fractions of PI for angles.

*/
public double calcCv(double Pus, double Pds, double rhous, double massFlowRate,
   double percentValveOpening) {
 // Sine of 3417 / 30.0
 double sineFactor = Math.sin(3417 / 30.0);

 // Calculate Cv
 double Cv = massFlowRate / (0.0457 * Math.sqrt(Pus * 100.0 * rhous) * sineFactor
     * Math.sqrt((Pus - Pds) / Pus) * percentValveOpening / 100.0);

 return Cv;
}
@EvenSol
Copy link
Collaborator

EvenSol commented Jan 18, 2025 •
edited
Loading

Thanks a lot for reporting this issue, @andr1976 ! I made a branch to work on this. I will ask you to review the suggested update:
https://github.com/equinor/neqsim/tree/1258-inconsistency-in-gas-valve-cv-calculation.

Maybe also @Sviatose could have a look at this issue?

Test need to be updated and more tests should probably be added
master...1258-inconsistency-in-gas-valve-cv-calculation#diff-563898cd3b4968c399a06ad911a381a9aa44fb0e7683e17e8b253a75f1303f08

@EvenSol EvenSol linked a pull request Jan 18, 2025 that will close this issue
first update of Cv calculation #1259
Merged
@andr1976
Copy link
Author

andr1976 commented Jan 23, 2025 •
edited
Loading

Wow @EvenSol quite some changes made. I can recognise N1 used for liquid from IEC / python fluids with Caleb's adjustment due to units, but the value of N9 = 0.03 for (m^3/hr, kPa) gas I cant recognise. I dont have the latest IEC, but internally we use N9 = 22.5 for m3/h / kPa units at standard conditions , and N9 = 24.6 for m^3/hr kPa K at 0 deg C (according to Caleb Bell)

@EvenSol
Copy link
Collaborator

EvenSol commented Jan 25, 2025 •
edited
Loading

Wow @EvenSol quite some changes made. I can recognise N1 used for liquid from IEC / python fluids with Caleb's adjustment due to units, but the value of N9 = 0.03 for (m^3/hr, kPa) gas I cant recognise. I dont have the latest IEC, but internally we use N9 = 22.5 for m3/h / kPa units at standard conditions , and N9 = 24.6 for m^3/hr kPa K at 0 deg C (according to Caleb Bell)

@andr1976 Thanks for pointing to this error. I was supposed to follow the same way of implementation as in the fluids library. It is now updated with the same value as in Calebs code (N9 = 2.46E1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

first update of Cv calculation equinor/neqsim
2 participants
@EvenSol @andr1976