-
Notifications
You must be signed in to change notification settings - Fork 35
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
Bladder updated #108
Bladder updated #108
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small changes, but one area to discuss with me in person.
("serosa of body of urinary bladder", "ILX:0739276"), | ||
("lumen of body of urinary bladder", "ILX:0739252"), | ||
("serosa of neck of urinary bladder", "ILX:0739277"), | ||
("lumen of neck of urinary bladder", "ILX:0739256"), | ||
("urethra", "ILX:0733022"), | ||
("urethra", "ILX:0733022", "UBERON:0000057"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally prefer to use the UBERON term ahead of ILX; the first one in the list after the name is always used (the rest are there in case we need to match other data). This is unless you have to match digitized contours which are annotated with the ILX term.
serosaOfurinaryBladder = findOrCreateAnnotationGroupForTerm(annotationGroups, region, get_bladder_term("serosa of urinary bladder")) | ||
serosaOfurinaryBladder.getMeshGroup(mesh2d).addElementsConditional(is_urinaryBladder_serosa) | ||
lumenOfurinaryBladder = findOrCreateAnnotationGroupForTerm(annotationGroups, region, get_bladder_term("bladder lumen")) | ||
lumenOfurinaryBladder.getMeshGroup(mesh2d).addElementsConditional(is_urinaryBladder_lumen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: capitalize start of each word in CamelCase: missed capital U in Urinary.
@@ -822,7 +831,7 @@ def generateUreters(region, nodes, mesh, ureterDefaultOptions,elementsCountAroun | |||
elementsCountAroundUreter, trackSurfaceUreter1, ureter1Position, trackSurfaceUreter2, | |||
ureter2Position, ureterElementPositionDown, ureterElementPositionAround, | |||
xBladder, d1Bladder, d2Bladder, nextNodeIdentifier, nextElementIdentifier, | |||
elementsCountUreterRadial, annulusMeshGroups = []): | |||
elementsCountUreterRadial, urinaryBladderMeshGroup, annulusMeshGroups = []): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, perhaps best to rename this function to generateUreterInlets (it previously hadn't mentioned mention Ureters which was why I got Mabelle to change it, but should clarify it's just the region where they enter the bladder).
Main issue I have is that the new argument urinaryBladderMeshGroup group doesn't fit the pattern of annulusMeshGroups.
I suggest you replace both with:
bladderMeshGroups (= bladder, neck) to set on annulus and first row of ostium i.e. ostiumMeshGroups
ureterMeshGroups (= ureter, or None if not defining) to pass as vesselMeshGroups on ostium
Basically I consider the final element on the ostium as it opens into the bladder as part of the bladder, but all the elements in the ostium are in ureter group (as you can make the ostium quite long with many elements which clearly become ureters).
This may not be what you're after, for example you may want them to be only bladder or ureter, not both for the final element. See me to discuss?
@@ -838,7 +847,7 @@ def generateUreters(region, nodes, mesh, ureterDefaultOptions,elementsCountAroun | |||
nodeIdentifier, elementIdentifier, (o1_x, o1_d1, o1_d2, _, o1_NodeId, o1_Positions) = \ | |||
generateOstiumMesh(region, ureterDefaultOptions, trackSurfaceUreter1, ureter1Position, ureter1Direction, | |||
startNodeIdentifier=nextNodeIdentifier, startElementIdentifier=nextElementIdentifier, | |||
ostiumMeshGroups=None) | |||
vesselMeshGroups=[[urinaryBladderMeshGroup]], ostiumMeshGroups=[urinaryBladderMeshGroup]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, vesselMeshGroups should only have a ureter term in them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue with ostium vessel groups.
for i in range(elementsCountAlong - 1): | ||
rowMeshGroups.append(copy.copy(vesselMeshGroups[v]) if vesselMeshGroups else []) | ||
rowMeshGroups += [ostiumMeshGroups] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will have broken the heart scaffold for human and rat which use common ostium.
I think it is right to include the last bit as being both bladder + neck AND ureter; the reason is that our primary use is to project data onto, and some of the data for ureter could/should project onto these elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to put the last layer of the ostium mesh only in one meshGroup. Since the ureters enter the bladder, and based on anatomy, make a slit on its surface. So, the elements around the ostiums are only part of the ureter and not the bladder. So, I think the right way is to set the
"vesselMeshGroups=[[ureterMeshGroup]]" and the "ostiumMeshGroups=[]"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We make anatomical groups to support fitting and registration and for that purpose, the elements in the inlet zone are as much ureter as bladder. If, for example, we had data points for the outside of the ureter, some of them would need to project onto those elements, and likewise for the bladder.
I'm more concerned about there being a second row of elements that are only ureter, so the bladder has parts that are not bladder... Ideally I'd have just one element along the ostium to reduce mesh complexity, but I admit it's hard to get around the corner without distortions. I'm reasonably happy with 1 element along using:
Ostium diameter 2.2
Ostium length 0.5
Vessel end length factor 2.0
I notice the ostium has a problem with multiple elements along over such a small length due to something I've done to limit wall thicknesses, but if we decide we need 2 elements to go around this curve we'd want both to be in both bladder and ureter groups. BTW do you have left and right ureter groups?
The changes to the ostium code do break the heart scaffold & need to be undone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good.
Hi Richard. I added some annotations and UBERON IDs to the bladder.