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

Stomach #132

Merged
merged 40 commits into from
May 6, 2021
Merged

Stomach #132

merged 40 commits into from
May 6, 2021

Conversation

mlin865
Copy link
Contributor

@mlin865 mlin865 commented Apr 21, 2021

No description provided.

Copy link
Member

@rchristie rchristie left a comment

Choose a reason for hiding this comment

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

Very good, but some fixes needed as in review comments.
Note the human stomach has uneven bunching up of elements on the lesser curvature. Rat stomach has some nodes in non-ideal positions on cardia limiting ridge adjacent to 6-way point on lesser curvature. I'll accept this PR without these fixes, but we need to fix them soon.
Note when I properly smoothed the human central path (as requested in review comment), the bunching was almost gone, so this may be enough.

self.assertAlmostEqual(surfaceArea, 2436.4955183926895, delta=1.0E-6)
result, volume = volumeField.evaluateReal(fieldcache, 1)
self.assertEqual(result, RESULT_OK)
self.assertAlmostEqual(volume, 1168.9418891341106, delta=1.0E-6)
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to test number of annotation groups, checking numbers of elements in each of them. See test_heart.py

def createEftWedgeCollapseXi2(self, collapseNodes):
'''
Create a tricubic hermite element field for a wedge element, where xi2 collapsed on xi1 = 1.
:return: Element field template
Copy link
Member

Choose a reason for hiding this comment

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

Describe argument collapseNodes, for this and other functions, with an example.


def createEftWedgeCollapseXi2(self, collapseNodes):
'''
Create a tricubic hermite element field for a wedge element, where xi2 collapsed on xi1 = 1.
Copy link
Member

Choose a reason for hiding this comment

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

Change to "Create a tricubic hermite element field for a wedge element collapsed in xi2."

... it can be collapsed in xi2 at all other ends of xi1 and xi3 depending on collapseNodes and future implementations; describe this with param collapseNodes.


def createEftWedgeCollapseXi2(self, collapseNodes):
'''
Create a bicubic hermite linear element field for a wedge element, where xi2 collapsed on xi1 = 1.
Copy link
Member

Choose a reason for hiding this comment

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

Note just xi1 = 1. see my comments for bicubic linear function.

@@ -249,6 +249,70 @@ def createEftWedgeXi1Zero(self):
assert eft.validate(), 'eftfactory_tricubichermite.createEftWedgeXi1Zero: Failed to validate eft'
return eft

def createEftWedgeCollapseXi1Quadrant(self, collapseNodes):
'''
Create a bicubic hermite linear element field for a wedge element, where xi1 collapsed on xi3 = 0 or xi3 = 1.
Copy link
Member

Choose a reason for hiding this comment

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

Not limited to xi3 = 0 or xi3 = 1. Can also be xi2 = 0 and xi2 = 1 depending on collapseNodes. Document param collapseNodes (see my later comments).

Comment on lines 53 to 61
[Node.VALUE_LABEL_VALUE, Node.VALUE_LABEL_D_DS1, Node.VALUE_LABEL_D_DS2, Node.VALUE_LABEL_D2_DS1DS2, Node.VALUE_LABEL_D_DS3, Node.VALUE_LABEL_D2_DS1DS3], [
[ [ 70.8, 72.3, 0.0 ], [ -15.1, -43.8, 0.0 ], [ 37.3, -18.7, 0.0 ], [ 2.2, 7.2, 0.0 ], [ 0.0, 0.0, 38.7 ], [ 0.0, 0.0, 4.5 ] ],
[ [ 51.1, 30.7, 0.0 ], [ -21.4, -33.7, 0.0 ], [ 33.9, -17.5, 0.0 ], [ -9.0, -4.8, 0.0 ], [ 0.0, 0.0, 41.4 ], [ 0.0, 0.0, 0.9 ] ],
[ [ 27.8, 2.7, 0.0 ], [ -27.7, -18.4, 0.0 ], [ 20.5, -27.0, 0.0 ], [-17.1, -3.5, 0.0 ], [ 0.0, 0.0, 40.9 ], [ 0.0, 0.0, -5.0 ] ],
[ [ -0.3, -5.8, 0.0 ], [ -28.0, -2.6, 0.0 ], [ 0.4, -25.7, 0.0 ], [-12.6, 3.7, 0.0 ], [ 0.0, 0.0, 32.3 ], [ 0.0, 0.0, -9.4 ] ],
[ [ -26.5, -2.9, 0.0 ], [ -20.6, 8.0, 0.0 ], [ -5.4, -19.9, 0.0 ], [ -5.6, 7.7, 0.0 ], [ 0.0, 0.0, 22.1 ], [ 0.0, 0.0, -6.8 ] ],
[ [ -40.6, 7.4, 0.0 ], [ -11.0, 12.5, 0.0 ], [ -10.9, -10.9, 0.0 ], [ -1.3, 7.9, 0.0 ], [ 0.0, 0.0, 17.6 ], [ 0.0, 0.0, -5.9 ] ],
[ [ -48.1, 21.0, 0.0 ], [ -6.4, 15.7, 0.0 ], [ -8.4, -4.0, 0.0 ], [ 0.1, 4.9, 0.0 ], [ 0.0, 0.0, 10.5 ], [ 0.0, 0.0, -3.1 ] ],
[ [ -52.9, 38.6, 0.0 ], [ -3.2, 19.3, 0.0 ], [ -11.2, -1.5, 0.0 ], [ -5.7, 0.1, 0.0 ], [ 0.0, 0.0, 12.0 ], [ 0.0, 0.0, 6.1 ] ] ] ),
Copy link
Member

Choose a reason for hiding this comment

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

These haven't been been fully smoothed - please do all the smoothing tools before committing central path parameters incl. update directions, side axes, side derivatives UNLESS you have a need for particular values or they have been fitted.
Same for rat.

'Gastro-esophagal junction position along factor': 0.35,
'Annulus derivative factor': 1.0,
'Use cross derivatives': False,
'Use linear through wall' : False,
Copy link
Member

Choose a reason for hiding this comment

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

When I use this option I get:
AssertionError: eftfactory_bicubichermitelinear.createEftWedgeCollapseXi1Quadrant: Failed to validate eft
It even crashed when tried with multiple elements through the wall ... investigate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be due to the missing "setEftScaleFactorIds(eft, [1], []) " in a section which requires scale factor -1. I have updated eftfactory_bicubichermitelinear.createEftWedgeCollapseXi1Quadrant to reflect that. I am able to generate the the scaffold with linear through wall with multiple elements through wall. Can you please try on your mchine and let me know if this is not resolved?

'Number of elements around esophagus': 8,
'Number of elements around duodenum': 12,
'Number of elements between annulus and duodenum': 6,
'Number of elements through wall': 1,
Copy link
Member

Choose a reason for hiding this comment

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

The annulus is not also supporting multiple elements through the wall. Do you want to update it to do so?
May be best to remove it until you're ready. There are issues of transitioning to fewer elements at each end which will be tricky.

Comment on lines 255 to 256
'Refine number of elements around': 1,
'Refine number of elements along': 1,
Copy link
Member

Choose a reason for hiding this comment

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

Merge these to Refine number of elements surface as they must be the same when you have tricky general linear maps.

'Number of elements around duodenum': 12,
'Number of elements between annulus and duodenum': 6,
'Number of elements through wall': 1,
'Number of radial elements in annulus': 1,
Copy link
Member

Choose a reason for hiding this comment

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

When set to 2 this does some funny stuff on the lesser curvature for the rat ... can you look into. May be an issue in annulus mesh which can be fixed later. May want to add an issue on github for it?
Should this be Number of elements across cardia? User doesn't know which annulus you're referring to.

Copy link
Member

@rchristie rchristie left a comment

Choose a reason for hiding this comment

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

Only minor comment is whether you need the annotation group "esophagogastric junction".
Great work!

@rchristie rchristie merged commit 019c411 into ABI-Software:master May 6, 2021
@mlin865 mlin865 deleted the stomach branch May 6, 2021 02:10
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.

2 participants