imcs-compsim / meshpy

A general purpose 3D beam finite element input generator
Other
9 stars 3 forks source link

Add helical beam basic geometry function #60

Closed davidrudlstorfer closed 3 months ago

davidrudlstorfer commented 3 months ago

To enable the usage of meshpy within my own input file creation repository it is helpful to create a simple helical beam base geometry. This pull request introduces a new helical mesh creation function with the following parameters (crucial for the helical geometry):

  1. Axis: Central axis of helix
  2. Axis point: Point on central axis which is does not have to be aligned with the bottom plane of the helical geometry
  3. Start point: Start point of the helical geometry. Also determines the radius of the geometry.
  4. Twist angle: Angle of helical geometry (also known as pitch angle)
  5. Either height of helix or number of turns: Within my repository the total height of the helix is an important quantity. As an alternative I also added the number of turns as a second parameterization option.

I tested the mesh creation function in general orientations and setups and everything worked as expected. If the general approach is correct I would also add a test.

davidrudlstorfer commented 3 months ago

During the changes of your review the following two questions came up:

  1. If the warning is printed with warnings.warn the following commandline output is written out:
/home/rudlstorfer/01_work/01_workspace/meshpy/meshpy/mesh_creation_functions/beam_basic_geometry.py:566: UserWarning: Radius of helix is 0 or twist angle is 90 degrees! Simple line geometry is returned!
  warnings.warn(

At the end there's always an unnecessary warnings.warn(. Have you encountered that problem?

  1. I use black v22.12.0 to format my code. Yesterday you started one pipeline run and it failed due to unformatted code. Are there any specific settings within MeshPy I haven't found?

    EDIT

  2. I've also directly added three test cases (simple helix, simple rotated and offset helix and straight line) - this was a very good idea! My initial implementation of the rotation had a bug which is solved with your proposed simple rotation! This is the reason for the changes within the reference files.

davidrudlstorfer commented 3 months ago

Thanks for your review/feedback. I've added everything and sorry for the long discussions. Locally all tests succeed.

isteinbrecher commented 3 months ago

Two more little details:

isteinbrecher commented 3 months ago

@davidrudlstorfer I just merged #65 which might require a reordering of the created sets in the reference file for this merge request.

davidrudlstorfer commented 3 months ago

I've now done the following things which finalize the PR from my side:

/home/rudlstorfer/01_work/01_workspace/meshpy/tests/testing_utility.py:81: UserWarning: Path to baci-release not found. Did you set the environment variable BACI_RELEASE?
  warnings.warn(
...sssss.ss.ss.ss.ss.....................................s................................................ssmesh_1 number of nodes: 13
mesh_1 number of elements: 6
Point couplings before "couple_nodes": 0
Point couplings after "couple_nodes": 6
.
----------------------------------------------------------------------
Ran 109 tests in 2.527s

OK (skipped=16)