Hyperelastic material

[romagnoliFer]

Hello Matthias,

First, pygccx is an excellent lib thanks to your hard work.

I had to implement the hyperelastic card for a project and I can share it by committing the modified source code into the official repository or somehow sending it to you. I have also written an example called beamhe.py

Let me know your thoughts.

Thank you

[FrozenWinterheart]

Hello,

thank you for your interest and contribution to pygccx.
I appreciate your effort and would like to see your code and your example.

You can either send a pull request, or if it's only a few files, post them here in the diskussion.
I will have a look at it and merge them in my code.

Greetings, Matthias

[romagnoliFer]

hyperelastic.zip

Attached are the files in a zip folder.

[FrozenWinterheart]

Thank you very much for your files. I included it in my local project.

Here are a few remarks:

1. Example file
   Your example runs, but didn't converge to the end. Is this correct?
   I ran the example without any modification with the same version of ccx: 'calculix_2.20_4win/ccx_static.exe.
   Here is the last converged step I get:
   
   We should provide a fully working example. Maybe you can improve it?

2. File hyperelastic.py, line 97 ff.
   Is there a reason why you compare the enum values to strings?
   Normally the enum itself is compared.
   Instead of

if self.type.value == 'MOONEY-RIVLIN' or self.type.value == 'NEO HOOKE' or self.type.value =='YEOH':

it would be better to write:

if self.type == HEELasticTypes.MR or self.type == HEELasticTypes.NH or self.type == HEELasticTypes.YE:

This way a static type checker can track it.
Or even better:

if self.type in [HEELasticTypes.MR, HEELasticTypes.NH, HEELasticTypes.YE]:

3. Naming of your enum class
   You named the class HEElastic, but it should be EHyperElastic.
   The "E" at the beginning indicates it's an enum class, to distinguish from a "regular" class. The rest is named like the material keyword. Compare elastic:
   Keyword: *ELASTIC
   Enum: EElastic

4. Naming of your enum members
   The naming of enum members should follow as close as possible the name of the keyword parameter.
   If possible, the enum name and (string)value should be equal to the parameter name.
   If the value contains a hyphen (-) or a blank, it is replaced in the name by an underscore.
   I.e.:
   MR = 'MOONEY-RIVLIN' will become MOONEY_RIVLIN = 'MOONEY-RIVLIN'
   NH = 'NEO HOOKE' will become NOE_HOOKE = 'NEO HOOKE'

Naming it this way is more clear to the user, especially if intellysense and autocompletion is used.

5. Unit tests
   As you can see under model_keywords/test, there exists a unit test file for each keyword class to test if the class is doing what it should.
   All possible edge cases should be tested (100% code coverage).

Do you want to do the refactoring and unit tests, or should I do it?

Greetings, Matthias

[romagnoliFer]

Thank you for all the feedback, they all make sense and I concur with them all. Later this week I will go thru all the comments, amend the code and get back to you. Thanks!

…

On Mon, Jul 10, 2023, 10:47 AM Matthias Sedlmaier ***@***.***> wrote: Thank you very much for your files. I included it in my local project. Here are a few remarks: 1. Example file Your example runs, but didn't converge to the end. Is this correct? I ran the example without any modification with the same version of ccx: 'calculix_2.20_4win/ccx_static.exe. Here is the last converged step I get: [image: grafik] <https://user-images.githubusercontent.com/63599939/252070640-c99bb3ff-3867-4a67-b388-6dac67970a7d.png> We should provide a fully working example. Maybe you can improve it? 2. File hyperelastic.py, line 97 ff. Is there a reason why you compare the enum values to strings? Normally the enum itself is compared. Instead of if self.type.value == 'MOONEY-RIVLIN' or self.type.value == 'NEO HOOKE' or self.type.value =='YEOH': it would be better to write: if self.type == HEELasticTypes.MR or self.type == HEELasticTypes.NH or self.type == HEELasticTypes.YE: This way a static type checker can track it. Or even better: if self.type in [HEELasticTypes.MR, HEELasticTypes.NH, HEELasticTypes.YE]: 1. Naming of your enum class You named the class HEElastic, but it should be EHyperElastic. The "E" at the beginning indicates it's an enum class, to distinguish from a "regular" class. The rest is named like the material keyword. Compare elastic: Keyword: *ELASTIC Enum: EElastic 2. Naming of your enum members The naming of enum members should follow as close as possible the name of the keyword parameter. If possible, the enum name and (string)value should be equal to the parameter name. If the value contains a hyphen (-) or a blank, it is replaced in the name by an underscore. I.e.: MR = 'MOONEY-RIVLIN' will become MOONEY_RIVLIN = 'MOONEY-RIVLIN' NH = 'NEO HOOKE' will become NOE_HOOKE = 'NEO HOOKE' Naming it this way is more clear to the user, especially if intellysense and autocompletion is used. 1. Unit tests As you can see under model_keywords/test, there exists a unit test file for each keyword class to test if the class is doing what it should. All possible edge cases should be tested (100% code coverage). Do you want to do the refactoring and unit tests, or should I do it? Greetings, Matthias — Reply to this email directly, view it on GitHub <#2 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGXVYHUDMZVVMWMDDNJD6MTXPQPZ7ANCNFSM6AAAAAA2C37XJ4> . You are receiving this because you authored the thread.Message ID: ***@***.***>