-
Notifications
You must be signed in to change notification settings - Fork 122
Minor improvements #176
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
Minor improvements #176
Conversation
|
Hello Yoel, I don't understand what the issue with the numerical check is. Can you provide a test EOS object that triggers this? I also don't the mathematial validity of your proposed formula update. It doesn't check for the same thing. The existing check checks that
Sincerely, |
|
Hi Caleb, Thanks so much for reviewing! I dug deeper into why I was getting the errors.
On a related issue, I believe the liquid fugacity coefficient estimated in the test case below might be too large (up to exp(4345) for some chemicals). I do not fully understand the topology of liquid fugacities using an equation of state and how such a large number is possible. I was wondering if I could consult with you on this matter? I am happy to open this as a separate issue on github. Test case: from thermo import PR78MIX
hydrocarbon_chemicals = ['H2', 'Methane', 'Ethane', 'Propane', 'n-Butane']
eos_mix = PR78MIX(
Tcs=[33.145, 190.564, 305.322, 369.89, 425.125],
Pcs=[1296400.0, 4599200.0, 4872200.0, 4251200.0, 3796000.0],
omegas=[-0.219, 0.01142, 0.0995, 0.1521, 0.201],
kijs=[[0.0, -0.0044, -0.0781, -0.1311, -0.397],
[-0.0044, 0.0, -0.0059, 0.0119, 0.0185],
[-0.0781, -0.0059, 0.0, 0.0011, 0.0089],
[-0.1311, 0.0119, 0.0011, 0.0, 0.0033],
[-0.397, 0.0185, 0.0089, 0.0033, 0.0]],
zs=[0.015151515151515152, 0.07575757575757576,
0.30303030303030304, 0.30303030303030304,
0.30303030303030304],
T=150,
P=75102339158.62407
)
print(f'{eos_mix.phi_l=}')
eos_mix.fugacities()
print(f'{eos_mix.lnphis_l=}')Outputs: Thank you! |
|
Hi Caleb, I found a test case which gives the division by zero error. It is a trivial case and I think it should be handled at a outer level on my side. It's when the EOS has no components: eos_mix = PR78MIX(
Tcs=[],
Pcs=[],
omegas=[],
zs=[],
T=298.15,
P=101325
)No need to change anything, just following up. Thanks, |
|
Hello Yoel, I set P_MAX_FIXED = 1e9 Pa in the flasher architecture to prevent flash calculations in this fantasy region. Other types of equations of state are applicable to them but not cubic equations of state. Thermodynamics math really pushes floating point math to its limits! Sincerely, |
|
@CalebBell, thanks for the valuable insight! Super useful. Please feel free to either merge the PR to remove the print statement or take care of later on your side. Thank you! |
This pull includes two minor updates:
Use multiplication instead of division. This prevents division by zero error for a very marginal case with unreasonable inputs (which ultimately leads to a different exception).Although the changes are small, they will help get some cases passing in BioSTEAM.
Thank you!