Skip to content

Conversation

@LeoSchmider
Copy link
Contributor

@LeoSchmider LeoSchmider commented Jul 19, 2025

Updates functions documentation, update unit display. Refactor viscosity to dynamic viscosity at least for function docs and descriptions, because ultimately giving user the possibility to make kinematic viscosity calculation is pretty cool and having molar density and molar mass already calculated should easy the implementation. Final goal of this is to implement Prandlt's number calculation because for transport phenomena study this is a cool metric to use correlations.

@se-schmitt
Copy link
Owner

Hi @LeoSchmider , thanks again for fixing the bugs! For EoSvs. EOS, I personally prefer the all caps version EOS (so I would like to stick with that).

Regarding the dynamic viscosity, I think it's a good idea to make the distinction! Maybe it would also make sense to add the alias const dynamic_viscosity = viscosity as well as a function kinematic_viscosity(model, p, T, x). Also, feel free to add the calculation of dimensionless numbers like the Prandtl or Reynolds number (and more...) to the package. That would be valuable! I'm happy to help there.

@LeoSchmider
Copy link
Contributor Author

LeoSchmider commented Jul 21, 2025

Okay, I will keep the EOS. I was thinking to directly do a crtl + f to change viscosity to dynamic_viscosity but the alias is also viable option, less risky :). I was thinking to say Reynolds number but think to add characteristic length and velocity which are not a purely thermodynamic parameters seems to be a bit out of scope of this library for me, but if you are okay with it, happy to help.

@se-schmitt
Copy link
Owner

Yeah, Reynolds number might not be very advantageous, you're right.. But Prandtl and Schmidt numbers would be good fits at least!

@LeoSchmider
Copy link
Contributor Author

I think you can merge this PR, for new functions implementation it might be better that it has his own PR.

@se-schmitt
Copy link
Owner

Ah, I saw you changed nearly all appearances of viscosity. I should have expressed myself more precisely. My idea was the following:

  • changing the wording in the docs (as you already did thoroughly)
  • adding the alias const dynamic_viscosity = viscosity and export it incl. modifying the docstring like:
"""
    viscosity(model::EntropyScalingModel, p, T, z=[1.]; phase=:unknown)
    dynamic_viscosity(model::EntropyScalingModel, p, T, z=[1.]; phase=:unknown)

Dynamic viscosity `η(p,T,x)` (`[η] = Pa s`).
"""
  • also the alias DynamicViscosityData could be added in the same manner for the sake of consistency
  • keeping viscosity as default (especially internally and for the tests, e.g. ϱT_dynamic_viscosity is not necessary)

Sorry for this misunderstanding! Hope this is not much extra work for you

@LeoSchmider
Copy link
Contributor Author

No no don't worry, you made your point cristal clear the first time but because I'm kind of a newbie with julia my thought process on this was that I was not sure on how to implement the alias without breaking all the tests and all the models. So my idea was given the fact that everything was going to be broken somehow I told myself it might be worth it changing all the instancies of viscosity into dynamic_viscosity, I understand it's a bit radical but don't worry I will change everything back and implement the alias.

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.

2 participants