-
-
Notifications
You must be signed in to change notification settings - Fork 17
Upgrade adapter to preCICE version 3 API #34
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
Conversation
Co-authored-by: Ishaan Desai <ishaandesai@gmail.com>
* Change ANSYS to Ansys * Change FLUENT to Fluent * Use ticks to indicate code
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.
The installation instructions for a UDF under https://ansyshelp.ansys.com/public/account/secured?returnurl=/Views/Secured/corp/v242/en/flu_udf/flu_udf_sec_compile_tui.html say: "Copy your source file (for example, myudf.c) to the source directory (src)." I think we should stay consistent with this approach and provide the sources under fluent-adapter/src
and then ask the user to copy them to the example directory. Therefore, we should avoid the code duplication of examples/Cavity2D/libudf/src
.
Additionally: Do you want to mention https://ansyshelp.ansys.com/public/account/secured?returnurl=/Views/Secured/corp/v242/en/flu_udf/flu_udf_sec_compile_tui.html in the README.md
?
a = 0.01 # Thickness [m] | ||
MI = a**4 / 12 # Area moment of inertia | ||
ME = 117e9 # Modulus of elasticity (117 GPa) | ||
ll = 1.0 # Beam length [m] |
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.
a = 0.01 # Thickness [m] | |
MI = a**4 / 12 # Area moment of inertia | |
ME = 117e9 # Modulus of elasticity (117 GPa) | |
ll = 1.0 # Beam length [m] | |
a = 0.01 # Thickness [m] | |
MI = a**4 / 12 # Area moment of inertia of square cross section | |
ME = 117e9 # Modulus of elasticity (117 GPa) of copper | |
ll = 1.0 # Beam length [m] |
better keep this information. Otherwise these number are just "magic" numbers.
|
||
dim = interface.get_mesh_dimensions(mesh_name) | ||
print("Mesh dimensions:", dim) |
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.
print("Mesh dimensions:", dim) |
let's remove this debug statement
print("coordinate array to be sent to set_mesh_vertices = {}".format(coords)) | ||
print("mesh_name sent to set_mesh_vertices = {}".format(mesh_name)) | ||
print("mesh_id sent to set_mesh_vertices = {}".format(mesh_name)) |
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.
print("mesh_id sent to set_mesh_vertices = {}".format(mesh_name)) | |
print("mesh_name sent to set_mesh_vertices = {}".format(mesh_name)) |
no?
examples/Cavity2D/CSMdummy.py
Outdated
forces = interface.read_data(mesh_name, force_name, vertexIDs, dt) | ||
forces = interface.read_data("beam", "Forces", vertexIDs, dt) |
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.
Generally ok, but why? I usually prefer to define mesh_name
somewhere at the top during initialization. At some point we will have to change "beam"
to "Beam-Mesh"
. If we use a variable mesh_name = "beam"
we only have to change this at one place and there is a lower risk to forget it. They way how it is currently written here is a bit more risky in this regard.
examples/Cavity2D/CSMdummy.py
Outdated
interface.write_data(mesh_name, displ_name, vertexIDs, displacements) | ||
interface.write_data("beam", "Displacements", vertexIDs, displacements) |
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.
See above.
examples/Cavity2D/CSMdummy.py
Outdated
if interface.requires_initial_data(): | ||
interface.write_data(mesh_name, force_name, vertexIDs, forces) | ||
interface.write_data(mesh_name, displ_name, vertexIDs, displacements) | ||
|
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.
Why did you remove this? Generally we should initialize data. If this does not work at the moment: Let's at least put a todo here.
@@ -0,0 +1,5 @@ | |||
CSOURCES= fsi_udf.c fsi.c | |||
HSOURCES= fsi.h | |||
FLUENT_INC= /lrz/sys/applications/ansys/v241/fluent |
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.
FLUENT_INC= /lrz/sys/applications/ansys/v241/fluent | |
FLUENT_INC= /path/to/fluent # TODO has to be provided by user |
Is #
the correct way to define a comment?
@@ -0,0 +1,5 @@ | |||
CSOURCES= fsi_udf.c fsi.c | |||
HSOURCES= fsi.h | |||
FLUENT_INC= /lrz/sys/applications/ansys/v241/fluent |
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.
FLUENT_INC= /lrz/sys/applications/ansys/v241/fluent | |
FLUENT_INC= /path/to/fluent # TODO has to be provided by user |
See above.
examples/Cavity2D/libudf/src/fsi.c
Outdated
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.
I think we should not duplicate the src files for the adapter here and just provide them under fluent-adapter/src
. Be careful: Don't delete this file (and the others) but copy-paste them over the ones in fluent-adapter/src
(I'm not sure if there are any changes you want to keep).
<relative-convergence-measure limit="1e-6" data="Displacements" mesh="beam"/> | ||
<relative-convergence-measure limit="1e-6" data="Forces" mesh="beam"/> |
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.
Why? This should not harm but I would like to know if this is necessary or not.
</participant> | ||
|
||
<m2n:sockets from="Fluent" to="CSMdummy"/> | ||
<m2n:sockets acceptor="Fluent" connector="CSMdummy" exchange-directory=".."/> |
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.
<m2n:sockets acceptor="Fluent" connector="CSMdummy" exchange-directory=".."/> | |
<m2n:sockets acceptor="Fluent" connector="CSMdummy"/> |
Does not harm but this is also not needed.
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.
Looks good to me. I did not test the adapter myself but it is confirmed to be working by @khalilhkiri.
No description provided.