initial commit of bone scaffold #239
Conversation
There was a problem hiding this comment.
@vickieshim - I have added some suggestion to clean up the current code. In addition, it would be good to get the annotations in once you have that ready. I would also suggest writing a unit test for bone1. You can look at tests/test_cylinder.py and come up with something similar for testing bone1. Let me know if you need any help or have any question.
| from scaffoldmaker.utils.shieldmesh import ShieldMesh3D | ||
| from scaffoldmaker.utils.spheremesh import SphereMesh, SphereShape | ||
| from scaffoldmaker.utils.zinc_utils import exnode_string_from_nodeset_field_parameters | ||
| from scaffoldmaker.utils.derivativemoothing import DerivativeSmoothing |
There was a problem hiding this comment.
Delete line if not planning to use DerivativeSmoothing
There was a problem hiding this comment.
Leaving it just in case
|
|
||
| class MeshType_3d_bone1 (Scaffold_base): | ||
| """ | ||
| Generates a solid cylinder using a ShieldMesh of all cube elements, |
There was a problem hiding this comment.
Ident to match indentation of start of docstring (""")
| 'Upper scale': 1.0, | ||
| 'Upper scale_Z': 1.0, | ||
| 'Lower scale': 1.0, | ||
| 'Lower scale_Z': 2.0 |
There was a problem hiding this comment.
Is 'Lower scale_Z' set as 2.0 for a specific reason?
| 'Number of elements across transition', | ||
| 'Number of elements along', | ||
| 'Shell element thickness proportion', | ||
| # 'Lower half', |
There was a problem hiding this comment.
Delete line if you are not planning to use it later.
| 'Refine', | ||
| 'Refine number of elements across major', | ||
| 'Refine number of elements along', | ||
| 'Upper scale', |
There was a problem hiding this comment.
Move these 4 scaling parameters ('Upper scale', 'Upper scale_Z', 'Lower scale', 'Lower_scale_Z') to before the "Refine" option to be consistent with the style in all our other scaffolds.
| hemisphere2.nodeId[elementsCountAcross[2] // 2][n2][n1] = nodesIdCylinderDistalEnd[count] | ||
| count += 1 | ||
|
|
||
| # # rangeOfRequiredElements = [[0, elementsCountAcross[0]], [0, elementsCountAcross[1]], [0, 1]] |
There was a problem hiding this comment.
Delete block o commented out code
| # # annotationGroup.createMarkerNode(nodeIdentifier, coordinates, boneCoordinatesValues) | ||
| # # nodeIdentifier += 1 | ||
|
|
||
| # smoothing = DerivativeSmoothing(region, coordinates) |
There was a problem hiding this comment.
Delete if not planning to use derivative smoothing
| rangeOfRequiredElements) | ||
|
|
||
| # *********************************************************************************************************************** | ||
| # top half of the hemisphere |
There was a problem hiding this comment.
The code for making the second hemisphere follows the same structure as the ones used in the first hemisphere, with some difference in parameters. Try combining them into one set of code with a loop instead of having two sets of similar code. It will reduce the length of the code and make your code look more elegant.
There was a problem hiding this comment.
Will try at the next iteration?
| @@ -0,0 +1,461 @@ | |||
| """ | |||
| Generates a solid bone using a ShieldMesh of all cube elements, | |||
| with variable numbers of elements in major, minor, shell and axial directions. | |||
There was a problem hiding this comment.
remove space at start of line
| options['Number of elements across major'] = 4 | ||
| if options['Number of elements across major'] % 2: | ||
| options['Number of elements across major'] += 1 | ||
|
|
There was a problem hiding this comment.
Delete space between lines
There was a problem hiding this comment.
To really make it a valid bone scaffold it should have 1 or more in-built parameter sets that make more realistic bone shapes, and these need annotations.
Furthermore it lacks a test to validate it works going forward.
Approving as it works, and completing it will be easier with it in the codebase.
No description provided.