Skip to content
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

Structural Mechanics Cases #7

Open
wants to merge 46 commits into
base: master
Choose a base branch
from
Open

Conversation

WeiqSun97
Copy link

This PR add necessary boxes to create structural mechanics cases

@WeiqSun97 WeiqSun97 self-assigned this May 28, 2021
@WeiqSun97 WeiqSun97 requested a review from roigcarlo May 28, 2021 08:25
Copy link
Member

@philbucher philbucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor, thx for the addition!

this.addInput("material_import_settings", "map"); // 1
this.addOutput("solver_settings", "map");
this.properties = {
"solver_type" : "Static",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"solver_type" : "Static",
"solver_type" : "static",

new syntax

"residual_relative_tolerance" : 0.0001,
"residual_absolute_tolerance" : 1e-9,
"max_iteration" : 10,
"rotation_dofs" : false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be variable, can you expose it like the domain size?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ricc wanted those to be modified directly from the properties

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, you know better :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the other GUIs this is automatically switched on depending on the user-defined elements. In this case, automating it would imply parsing the mpda (which we already do) and go through all the element types in the elements section. Not sure if this will be possible.

What is not clear to me is how we can select this field from the properties (e.g. the properties of a shell element are almost identical to that of a 2D plane stress case without rotations).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is similar as the CL problem that appeared above. As @rubenzorrilla says we need to extract more info from the mpda, which is not a problem but also need to dynamically generate nodes based on inputs, which is something we are missing at the moment (but possible).

I would leave it as is for this PR and make a separate one to implement those changes.

Copy link
Member

@roigcarlo roigcarlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@WeiqSun97 WeiqSun97 closed this May 29, 2021
@WeiqSun97 WeiqSun97 deleted the StructuralMechanicsWorkFlow branch May 29, 2021 09:33
@WeiqSun97 WeiqSun97 restored the StructuralMechanicsWorkFlow branch May 29, 2021 09:34
@WeiqSun97 WeiqSun97 deleted the StructuralMechanicsWorkFlow branch May 29, 2021 09:35
@WeiqSun97 WeiqSun97 restored the StructuralMechanicsWorkFlow branch May 29, 2021 09:35
@WeiqSun97 WeiqSun97 deleted the StructuralMechanicsWorkFlow branch May 29, 2021 09:36
@WeiqSun97 WeiqSun97 restored the StructuralMechanicsWorkFlow branch May 29, 2021 09:42
@WeiqSun97 WeiqSun97 reopened this May 29, 2021
@WeiqSun97 WeiqSun97 force-pushed the StructuralMechanicsWorkFlow branch from 7cff740 to 50654a7 Compare May 29, 2021 12:15
Comment on lines +56 to +57
<script type="text/javascript" src="js/nodes/output_processes/gid_output_structural.js"></script>
<script type="text/javascript" src="js/nodes/output_processes/vtk_output_structural.js"></script>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need specific nodes for the structural output?

Copy link
Member

@roigcarlo roigcarlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of comments in the usage of

  • AssignScalarVariableToConditionsProcess
  • AssignVectorVariableProcess

Comments apply to both, but are basically the same. Any of the solutions would work, but I prefer if you could define them through the process_parser.js and update the examples accordingly.

Once this is addressed, I se no other reason to hold it back 👍

js/nodes/processes/load_process_list_pressure.js Outdated Show resolved Hide resolved
js/nodes/processes/load_process_list_pressure.js Outdated Show resolved Hide resolved
Copy link
Member

@roigcarlo roigcarlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 If no one else have anything to add I think we can merge! Well done @WeiqSun97

@@ -1,7 +1,7 @@
class MaterialWriter {
constructor()
{
this.material_file_name = this.addWidget("string","Materials Filenae", "Materials");
this.material_file_name = this.addWidget("string","Materials Filenae", "Materials.json");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.material_file_name = this.addWidget("string","Materials Filenae", "Materials.json");
this.material_file_name = this.addWidget("string","Materials Filename", "Materials.json");

Typo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are commenting on an outdated file, as the specific file material_writer.js has been modified in this PR.

@roigcarlo
Copy link
Member

@Rbravo555 can we merge this? I would like to merge some changes on my side and I am afraid they can create conflicts.
I'd be more comfortable if the conflicts are on my side of the PR

@Rbravo555
Copy link
Member

@Rbravo555 can we merge this? I would like to merge some changes on my side and I am afraid they can create conflicts. I'd be more comfortable if the conflicts are on my side of the PR

If you really need to merge your changes now, go ahead. But, how about we discuss this in a short meeting tomorrow morning? Afterwards, we can decide.

@roigcarlo
Copy link
Member

No no, no hurry, but since this have been untouched for a long time I tought it was finished.

Let's discuss it tomorrow 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants