Conversation
…odification of the name of the file
…he name of the file
…+ modified the test suite name
WPK4FEM
left a comment
There was a problem hiding this comment.
Hi Marjan,
Thank you for all the good stuff here and for putting the used labels in a file and reusing them. I have some small comments in the test file setup and a remark about the direct shear picture. The code for the UI I have tried to understand, but failed due to my lack of experience with such programming. Luckily you have asked Richard to look at that too.
Regards, Wijtze Pieter
There was a problem hiding this comment.
I wonder about the tau_xy indication in the picture. tau_xy is the shear stress that would appear on the contact of the 2 moving boxes in the picture. The test works with either a force or a prescribed displacement on the side which leads to a shear stress in the contact zone.
There was a problem hiding this comment.
I'll try to fix that in the next PR.
| "PERMEABILITY_YY" : 4.5e-30, | ||
| "PERMEABILITY_XY" : 0.0, | ||
| "DYNAMIC_VISCOSITY" : 8.9e-07, | ||
| "THICKNESS" : 1.0 |
There was a problem hiding this comment.
Thickness is not needed in this plane strain computation.
Tables is empty and probably redundant.
| "model_part_name" : "PorousDomain.Top_load", | ||
| "variable_name" : "NORMAL_CONTACT_STRESS", | ||
| "active" : [true,false], | ||
| "value" : [0.0,0.0], | ||
| "fluid_pressure_type" : "Uniform", | ||
| "table" : [1,0] | ||
| } |
There was a problem hiding this comment.
As we have verically constrained the movement of the top nodes, this load disappears straight into the supports. We might omit it. The vertical stress comes from the next apply_initial_uniform_stress_field process.
| "nodal_nonhistorical_results" : [], | ||
| "gauss_point_results" : ["CAUCHY_STRESS_TENSOR","VON_MISES_STRESS","MEAN_EFFECTIVE_STRESS","ENGINEERING_STRAIN_TENSOR"] | ||
| }, | ||
| "point_data_configuration" : [] | ||
| }, | ||
| "output_name" : "gid_output/output" | ||
| } |
There was a problem hiding this comment.
Please remove the empty placeholders, I think we do not use them.
applications/GeoMechanicsApplication/python_scripts/element_test/test_direct_shear/mesh.mdpa
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/element_test/plots.py
Show resolved
Hide resolved
rfaasse
left a comment
There was a problem hiding this comment.
Nicely done, it works nicely and looking at the code it was added in a generic way (I expect adding the oedometer test would not be too difficult due to how extendible its setup is👍 ).
I have some suggestions, but nothing major. For me it's also still quite difficult to review due to being unfamiliar with the framework.
...GeoMechanicsApplication/python_scripts/element_test/test_direct_shear/ProjectParameters.json
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/element_test/assets/icon.ico
Show resolved
Hide resolved
|
|
||
| def update_middle_maximum_strain(self, maximum_strain): | ||
| pattern = r'\$middle_maximum_strain\b' | ||
| prescribed_middle_displacement = (-maximum_strain / 2) / 100 |
There was a problem hiding this comment.
Just for my understanding, is this the average between the max and 0? To me it's not clear why we need a minus sign and why we need to divide by 100.
There was a problem hiding this comment.
Ah.. yeah probably not a very clear coding from my side :D
The maximum strain will be taken from the user as an input in percentage. We need to divide by 100 to get the value. And then a minus sign (in Triaxial) was because we were applying compression. But here in the Direct Shear, I don't think it matters much which direction it gets applied because it is applied in X direction and we don't have any condition on the other side to make it matter from which side it should be applied. I won't change it now but will take a note to remember to maybe generalize it later.
Also, about the averaging, yes indeed. When setting up the Direct shear test, you have to apply the shear stress to the top half part of the soil model. We basically mimic that by applying the half amount of the shear strain to the middle points. Hope this is clear.
There was a problem hiding this comment.
The shear strain and shear stress ( like any other strain or stress ) are uniform in these tests. To get such a uniform strain the motion of the nodes is prescribed such that the bottom nodes do not move, the top nodes move all with the same distance and the nodes at half the height of the specimen move half the distance of the top nodes.
There was a problem hiding this comment.
Clear, I missed the '%' that is clearly there in the GUI and the rest is clear from both your explanations
applications/GeoMechanicsApplication/python_scripts/element_test/plots.py
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/element_test/run_simulation.py
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/element_test/ui_runner.py
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/element_test/ui_runner.py
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/element_test/test_direct_shear/mesh.mdpa
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/element_test/test_direct_shear/mesh.mdpa
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/element_test/ui_builder.py
Outdated
Show resolved
Hide resolved
… the axis range once sigma_1 and sigma_3 are close together
…ult maximum_strain to 20%
…ion number to the title
rfaasse
left a comment
There was a problem hiding this comment.
Thanks for processing the review comments! I have a few comments left, but most of them are not for changes that need to be done right now.
I think after this v0.1.0 release, we should have a discussion with the team to determine what exactly the direction and our requirements are for this GUI and the extensions we want to make. There are for example design patterns regarding GUIs to make it more maintainable and split the logic (of running and calculating things) with displaying everything (these are patterns like Model-View-Control or Model-View-Viewmodel).
Right now I don't have the overview (both on direction and the current code) on if it would be necessary to do some restructuring there or if it's already fit for everything we want it to be able to do (mainly in the future).
For now, well done, I think we're almost there!
| self.test_type_menu = ttk.Combobox( | ||
| parent, | ||
| textvariable=self.test_type_var, | ||
| values=["Drained"], |
There was a problem hiding this comment.
I'm not too sure if we need to have this drop-down with a single value (since we've only got 'Drained'). Given the time-frame, let's leave it as is for now, but in general my opinion it's better not to add more functionality than needed in the future.
There was a problem hiding this comment.
This was requested and it is to show the user what type of test this is. For now we have "Drained" only, but more options will have to be added later, like: "Undrained", etc...
|
|
||
|
|
||
|
|
||
| def show_license_agreement(): |
There was a problem hiding this comment.
Not for now, but I'd prefer to have this in a separate plain resource txt file somewhere in a later PR
| if not os.path.exists(LICENSE_FLAG_PATH): | ||
| show_license_agreement() |
There was a problem hiding this comment.
This means it doesn't really matter what's in the file right? I'm not sure if that would matter. Of course people can circumvent this by just creating that file themselves, but I don't think that should be an issue, since they also would just have access to the python files, so they can do what they want
There was a problem hiding this comment.
I mean... Yes :D
| license_window.destroy() | ||
|
|
||
| def decline(): | ||
| messagebox.showinfo("Exit", "You must accept the license agreement to use this software.") |
There was a problem hiding this comment.
If you close the license popup, the application will crash instead of showing this exit message (not a blocker, but maybe something to fix in the future)
| license_window.destroy() | ||
|
|
||
| def decline(): | ||
| messagebox.showinfo("Exit", "You must accept the license agreement to use this software.") |
There was a problem hiding this comment.
Also it's interesting you can decline it after you already accepted it before, but that's also fine for now in my opinion
|
|
||
| def update_middle_maximum_strain(self, maximum_strain): | ||
| pattern = r'\$middle_maximum_strain\b' | ||
| prescribed_middle_displacement = (-maximum_strain / 2) / 100 |
There was a problem hiding this comment.
Clear, I missed the '%' that is clearly there in the GUI and the rest is clear from both your explanations
📝 Description
The GUI for the Element test has already been created. The triaxial test has also been added earlier. This issue aims at creating a direct shear test and also adding it to the GUI and being able to see the results plots.
🆕 Changelog