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

Rescale derivatives for annulus mesh #100

Merged
merged 19 commits into from
Nov 18, 2020
Merged

Conversation

mlin865
Copy link
Contributor

@mlin865 mlin865 commented Nov 12, 2020

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.

Some small fixes for you to do.
I'll shortly make a pull request to your branch to fix some bits of annulus mesh.

Comment on lines 196 to 198
'Ostium position around': 0.67, # should be on the dorsal part (> 0.5)
'Ostium position down': 0.83,
'Number of radial elements on annulus': 2,
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind fixing these names...
Key is that all of these options are about the Ureter, which is built with an ostium + annulus. Also, my 'standard' is to always start numbers of elements with 'Number of elements' + qualifiers, and in the code elementsCount~.
Can you please rename these to:
Ureter position around
Ureter position down
Number of elements ureter radial
& in the code update variables they are assigned to appropriately.

@@ -343,7 +352,7 @@ def generateBaseMesh(cls, region, options):
ostiumOptions = options['Ureter']
ostiumDefaultOptions = ostiumOptions.getScaffoldSettings()
elementsCountAroundOstium = ostiumDefaultOptions['Number of elements around ostium']
elementsCountAnnulusRadially = options['Number of elements radially on annulus']
elementsCountAnnulusRadial = options['Number of radial elements on annulus']
Copy link
Member

Choose a reason for hiding this comment

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

Rename to elementsCountUreterRadial, as earlier.

Comment on lines 356 to 357
ostiumPositionAround = options['Ostium position around']
ostiumPositionDown = options['Ostium position down']
Copy link
Member

Choose a reason for hiding this comment

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

Renam ureterPosition~, and other variables such as ostium1Position --> ureter1Position.

trackSurfaceOstium2, ostium2Position, ostiumElementPositionDown,
ostiumElementPositionAround, xBladder, d1Bladder, d2Bladder,
nextNodeIdentifier, nextElementIdentifier, elementsCountAnnulusRadially,
def generateOstiumsAndAnnulusMeshOnBladder(region, nodes, mesh, ostiumDefaultOptions,elementsCountAround,
Copy link
Member

Choose a reason for hiding this comment

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

Rename to generateUreters.

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.

Looks good, thanks.

@rchristie rchristie merged commit ed71ce3 into ABI-Software:master Nov 18, 2020
@mlin865 mlin865 deleted the ureters branch November 19, 2020 00:03
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