-
Notifications
You must be signed in to change notification settings - Fork 9
Move input file & testing to fourcipp #330
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
base: main
Are you sure you want to change the base?
Move input file & testing to fourcipp #330
Conversation
b05b409
to
ffa452e
Compare
@isteinbrecher today I also made a lot of progress on this PR and it's already looking good I am now able to execute the following script, i.e., create a mesh + input file from scratch with (almost) everything (except the nurbs patches), read an existing input file as a pure input file and not convert it to a mesh, convert an existing input file into a MeshPy mesh and write it out again. I think the overall changes are coming to a (more or less) final form. There are still a lot of open ToDo's. But I think going forward it would make sense if you could quickly look over the changes or if we discuss some of it in the upcoming days/weeks. I am curious about your opinion. Script
|
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.
Thanks for the work @davidrudlstorfer (and please don't get discouraged by my many comments, there are just a lot of changes).
One thing I am not exactly sure, I thought we agreed to not have any dump
methods in the mesh items (#326 (comment)) - this would help to solve all the parts I marked that are currently in core
but should move somewhere to four_c
.
Regarding your example script, I like the approach with from_4C_yaml
(will take some time getting used to, but I think it makes sense). I am thinking if it makes sense to not have this as a class method but as a free function load_4C_yaml
, and always return two variables (otherwise we will have a lot of additional if
s in testing).
# also test second approach to load input file (not as mesh)
# input_file2 = FourCInputFile().from_4C_yaml(input_file_path="test.4C.yaml", convert_input_to_mesh=False)
input_file2, _ = load_4C_yaml("test.4C.yaml", convert_input_to_mesh=False)
# also test third approach to load input file (and convert to mesh)
# input_file3, mesh2 = FourCInputFile().from_4C_yaml(input_file_path="tests/reference-files/4C_input_solid_cuboid.4C.yaml", convert_input_to_mesh=True)
input_file3, mesh2 = load_4C_yaml("tests/reference-files/4C_input_solid_cuboid.4C.yaml", convert_input_to_mesh=False)
I think it makes sense to sit together and talk about some of the points. I will let you know what dates are good for me.
@@ -156,9 +156,6 @@ def set_default_values(self): | |||
# match. | |||
self.allow_beam_rotation = True | |||
|
|||
# Import meshes as pure dict or import the geometry. | |||
self.import_mesh_full = False |
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.
🎉 nice to see this monster go!
@@ -41,16 +41,15 @@ def __init__(self, nodes=None, string_pre_nodes="", string_post_nodes="", **kwar | |||
self.string_pre_nodes = string_pre_nodes | |||
self.string_post_nodes = string_post_nodes | |||
|
|||
def dump_to_list(self): | |||
"""Return a list with the items representing this object (usually a | |||
# ! TODO fix this export with a dict |
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.
Is there a reason we can not do this straight away? Maybe if we apply my previous comment this would resolve itself? Or am I misunderstanding something here?
{ | ||
"type": "NODE", | ||
"node_id": node.i_global, | ||
"d_type": self.geometry_set_names[self.geometry_type], |
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.
Pre evalutare
@@ -148,7 +148,7 @@ def read_dbc_monitor_file(file_path): | |||
|
|||
|
|||
def add_point_neuman_condition_to_input_file( | |||
input_file: _InputFile, | |||
input_file: _FourCInputFile, |
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 this has to be a Mesh
.
@@ -124,6 +124,15 @@ def check(self): | |||
"specified on how to handle them!" | |||
) | |||
|
|||
def dump_data(self) -> _Dict: |
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 would not put this here. This is 4C specific and should not be in core
in my opinion.
return dict | ||
|
||
|
||
def add_items_to_dict(dict, section_name, items): |
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.
Side node, but we could mark functions like this as private _add_items_to_dict
.
def convert_mesh_to_dict( | ||
mesh: _Mesh, | ||
*, | ||
global_start_indices: dict = {_Node: 0, _Element: 0, _Material: 0, _Function: 0} |
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 a better approach would be to provide an empty dict here and do all lookups like
global_start_indices.get(xxx, default=0)
Then, for example if I only want to provide a starting index for elements, because everything else starts with 0 in my solver, I can simply pass a dict with a single entry for the element starting index and the others indices will automatically be 0.
if item_type == _Node: | ||
indices[_Node] = len(self.sections.get("NODE COORDS", [])) | ||
|
||
# elements |
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 think the comments here are not needed (also for nodes, materials, ...)
xml_file.write(self.nox_xml) | ||
# materials | ||
elif item_type == _Material: | ||
indices[_Material] = max( |
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.
Very nice how simple this has become.
): | ||
self.add_section(args[0], **kwargs) | ||
return | ||
return input_file, mesh |
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 am not sure if I missed this, but do we ever delete the mesh related information from input_file
?
This evening I finally found some time to really tackle this larger move (following the initial discussion and plan in #321) to fourcipp
The present changes are a very rough WIP and are not ready yet (I hope that this is clear :D)
The first draft now proposes the completely overhauled FouCInputFile structure which is based on fourcipp.
Some points:
The missing points
dump_to_list
remnants (except for Nurbs -> I have no overview on how to approach this the best way)_if_fourcipp_is_available
occurences (except for Nurbs -> I have no overview on how to approach this the best way)