-
Notifications
You must be signed in to change notification settings - Fork 73
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
Added NS incompressible and Euler equation. Also fixed tessellation, so now will work on windows via docker or pip #30
base: main
Are you sure you want to change the base?
Conversation
(only docker tested, however supposed to work with pip too (will try to test in the future)) and cannot calculate sdf derivative in windows currently
added notes for windows and pip fixes
I fixed tessellation, so now will work on windows via docker or pip. I used mesh_to_sdf instead of pysdf |
Looking forward for reviews, since our team cannot upgrade to modulus:23.05 due to missing pysdf library |
README.md
Outdated
pip install trimesh | ||
pip install mesh_to_sdf | ||
``` | ||
Tesselation will work because insted of using pysdf I used mesh_to_sdf (I have not yet implemented sdf derivatives yet, so some function will not work on windows (It's only a few functions for visualisation difference with real data (as far as I know))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor typo, should be instead
modulus/sym/eq/pdes/navier_stokes.py
Outdated
@@ -339,6 +339,90 @@ def __init__(self, T, dim=3, time=True): | |||
normal_x * T.diff(x) + normal_y * T.diff(y) + normal_z * T.diff(z) | |||
) | |||
|
|||
class NavierStokesIncompressible(PDE): | |||
'''Cystom implementation of incompressible NS eqyastyans''' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be custom and equations
modulus/sym/eq/pdes/navier_stokes.py
Outdated
|
||
|
||
class Euler(PDE): | ||
'''Cystom implementation of compressible Euler eqyastyans''' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo as above
modulus/sym/eq/pdes/navier_stokes.py
Outdated
|
||
# density | ||
if isinstance(rho, str): | ||
rho = Function(rho)(*input_variables) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this incompressible, then surely rho must be a constant? Therefore raising an exception would make more sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably correct, I will fix it
fixed typos in README.md
fixed typos and added Exeption for incompresiible rho
Implemented all suggestions |
modulus/sym/eq/pdes/navier_stokes.py
Outdated
|
||
# density | ||
if isinstance(rho, str): | ||
raise Exception("rho myst be number") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo must
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
I need to check the code when I get a chance, but how is this different from calling the Navier Stokes with a constant rho? |
In classical CFD, for better results, incompressible form is used for low Mach numbers. I was able to achieve better results with an incompressible equation for PINNs than with a compressible equation but with constant rho |
Just for the record I can’t review or merge your work. |
If you can it would be good to have an example of your changes working. You could use the lid driven cavity for the incompressible formulation. I’m not sure if there is a compressible example with shockwaves to show off the Euler equations. |
Can you work via pip? |
e.g modulus-sym\examples\geometry\tessellated_example.py |
It appears on on regular tessellation too. This warning can be ignored |
Hi @chaous , sorry for the delay in getting attention to this PR. Thanks for the contribution! Can we split this PR into two? One that adds equations and the other that adds changes to tessellation? Reviewing the tessellation might take some time, but we can get the equations added much sooner. Also, if you can add an example to go along with your equation additions that can demonstrate potential speed-up/improvement in accuracy, that would be helpful. |
I split my pull request into two and added examples |
In some cases it is better to use incompressible NS and Euler equation uses way less memory