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

Cylinder/rim #111

Merged
merged 10 commits into from
Dec 14, 2020
Merged

Cylinder/rim #111

merged 10 commits into from
Dec 14, 2020

Conversation

elias-soltani
Copy link
Contributor

I would like to add the ability to add rim elements to the cylinder mesh, please.

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.

It's looking great, but there are some issues.
Something in this commit has broken ventricles3 pig parameters scaffold; can you look into it?
It has some overly large d1 derivatives in the centre (and sometimes nodes around it on the minor axis direction) which causes problems, most visible if you have 8 or more elements across major and minor. (Smooth derivatives function makes them look more appropriate, but we need to get them right in the script, not rely on calling this function.)

Comment on lines -132 to +134
tx, td1 = sampleCubicHermiteCurves(
[ self.px[n3][n2a][n1b], self.px[n3][n2c][n1c] ], [ [-self.pd3[n3][n2a][n1b][c] for c in range(3)], [ (self.pd1[n3][n2c][n1c][c] + self.pd3[n3][n2c][n1c][c]) for c in range(3) ] ], 2, arcLengthDerivatives = True)[0:2]
# tx, td1 = sampleCubicHermiteCurves(
# [ self.px[n3][n2a][n1b], self.px[n3][n2c][n1c] ], [ [-self.pd3[n3][n2a][n1b][c] for c in range(3)], [ (self.pd1[n3][n2c][n1c][c] + self.pd3[n3][n2c][n1c][c]) for c in range(3) ] ], 2, arcLengthDerivatives = True)[0:2]
Copy link
Member

Choose a reason for hiding this comment

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

Why are these commented out here & in subsequent lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not have points between the rim and triple points at this stage, and they will be created later on. Also, I am not using it for finding the triple point. I can remove the commented lines?

@elias-soltani
Copy link
Contributor Author

Thanks for reviewing the code all requested changes were done.

@rchristie rchristie merged commit b545482 into ABI-Software:master Dec 14, 2020
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.

I have approved and pulled the changes; a subsequent development will fix the element layout as discussed.

@elias-soltani elias-soltani deleted the Cylinder/rim branch December 16, 2020 00:51
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