-
Notifications
You must be signed in to change notification settings - Fork 248
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
MedApplication #11419
MedApplication #11419
Conversation
The med lib is this one https://docs.salome-platform.org/latest/dev/MEDCoupling/developer/med-file.html? |
Link is broken :P http://www.salome-platform.org/downloads/current-version BTW it is LGPL if anyone wonders |
I will take a look ASAP (I started yesterday my holydays during for one month) |
no worries, I am not in a hurry |
yes exactly |
applications/MedApplication/custom_python/add_custom_utilities_to_python.cpp
Show resolved
Hide resolved
for (auto geom_2 : rModelPart2.Geometries()) { | ||
if (have_same_nodes(geom_1, geom_2)) { | ||
CheckGeometriesAreEqual(geom_1, geom_2); | ||
goto here; |
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.
break
?
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 pretty sure I tried, but break
IIRC works only for the inner loop
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.
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.
Unless I'm missing something, you're just breaking from the inner loop, not the outer one.
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.
Seems so, but I also know that implementing this took me a while (first time I eve used goto
)
Since I am a bit out of the loop rn I will leave it as is and revisit it next time I develop
check_geoms(rModelPart1, rModelPart2); | ||
check_geoms(rModelPart2, rModelPart1); |
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.
it seems to me that have_same_nodes
and CheckGeometriesAreEqual
are commutative, so the second check (check_geom(rModelPart2, rModelPart);
) is unnecessary.
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.
not sure
ModelPart1 could have less geometries than ModelPart2, which would only be detected in the second call
Checking if the geometries are the same in a modelpart is quite nasty :/
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.
true, but you're already checking whether the two ModelPart
s have the same number of geometries in line 121. Anyway, performance in a test isn't that important so this is just a minor comment.
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.
hm good point
I will think about this again next time I develop, I remember that I thought long about this
applications/MedApplication/tests/med_files/hexahedral_8N/create_mesh.py
Outdated
Show resolved
Hide resolved
applications/MedApplication/tests/med_files/tetrahedral_4N/create_mesh.py
Outdated
Show resolved
Hide resolved
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.
Some comments
Thanks guys for the reviews I will address the comments next week so that we can proceed Is there anyone interested in testing this? |
Not now, but maybe in the future |
ping @matekelemen do you have more comments? I would like to merge this so that I can continue to add support for sub-meshes :) |
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.
Nope, just the minor nitpicking comments from earlier. Thanks the idea and implementation, I'm planning to use it but I have to get familiar with Salome first!
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.
Whoops, I meant to approve :'(
Blocking to prevent accidental merge? 👀😂 I will have another look at you comments before merging, thanks! |
@philbucher alright I'm about try it rn, but I'd need support for submodelparts. Any idea how difficult it is to implement that (or have you already started working on it)? |
great! Yes I already started, but then I removed it again bcs it grew a lot. (for one PR) Lets merge this and then I can push my draft in the next days. I am guessing you only need the reading? Writing is a bit more tricky, sadly the med-lib is not very user-friendly, it is pretty confusing when it comes to submodelparts |
How does your model look like? Do you only have one level of submodelparts? Or e.g. also SubSubModelParts |
Yep, reading is fine for now. In fact, I don't think we'll ever want to output MED files. Should we ever manage to finally break away from MDPAs, it'd be best to choose an industry-wide standard format rather than yet another application-specific one.
For now, yes. I have a volume mesh with parts of its surfaces in subpartitions. I'm fine with taking baby steps and initially supporting 1-depth models exclusively, but we should only merge once we support arbitrarily nested models. Can you plz open a follow-up PR with what you have so far? Maybe I can piggyback off your work and help you with the rest of the impl. |
I am honestly not sure this exists 😅. Do you know?
And do you have entities that are part of more than one SubModelPart?
Yes, I will do that in a few days. In the meantime, do you (roughly) know how the med-lib stores the mesh? Especially do you know what groups and families are? |
merging to avoid blocking the developments Thanks for the comments everYbody! |
I sure hope there is one, though I have to admit that I have next to no experience with proper modelling and meshing.
Polytopes? No. Vertices? Definitely.
Nope, this is the first time I heard about MED, and I'll have to look everything up. |
I think so too, afaik it is only used by some older CFD solvers, i.e. not sth we should rely on exclusively
I dont think they include any mesh
I think this might be a good option, even though not the most user-friendly. At least it uses hdf under the hood which is nice
Ok thats fine, enabling this is not too difficult :)
Ok no worries |
This PR adds the first version of the interface to the MED-library (which is the mesh/data format used by Salome and Code_Aster). No external source code is added, the MED-library must be installed/compiled separately before this new App can be compiled. I.e. same as we include Trilinos or Metis
This is a full replacement of the KratosSalomePlugin, as then the med-files can be read directly.
Important missing feature is the support for SubModelParts, which I will implement in a follow-up PR. So far it can read and write Main-Modelparts, I tested it with the large examples from my thesis.
Also important to mention is that I work exclusively with Geometries, Elements and Conditions need to be created afterwards, as we also intend to use for other workflows. For this I would use #11370