Skip to content

Conversation

NiklasVin
Copy link
Collaborator

I added a solid fake implemented in Python to the perpendicular flap tutorial, similar to the already existing fluid-fake.

@MakisH
Copy link
Member

MakisH commented Jun 24, 2024

Is this ready for review?
Don't forget to add it into the README of the tutorial. I can generate some of the plots when reviewing.

I will fix the markdownlint, it is failing also elsewhere.

@MakisH
Copy link
Member

MakisH commented Jun 27, 2024

@NiklasVin please rebase/merge from develop, the markdownlint issue is now fixed.

@NiklasVin
Copy link
Collaborator Author

Is this ready for review? Don't forget to add it into the README of the tutorial. I can generate some of the plots when reviewing.

I will fix the markdownlint, it is failing also elsewhere.

Yes, now this should be ready for review.

@MakisH
Copy link
Member

MakisH commented Jul 2, 2024

@NiklasVin can you please enable edits by maintainers? It's a checkbox on the right.

Copy link
Member

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

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

I checked the case with fluid-openfoam. Generally runs and VTK output looks good. Only a minor point is left + some readability comments.

Comment on lines 10 to 13
# first, get displacement independent of x, only dependent on y and t
# maximal displacement at flap tip should be 0.5
# initially, flap's displacement is 0
x_displ = np.minimum(((np.sin(3 * np.pi * t + np.arcsin(-0.95)) + 0.95) / 8) * y / flap_tip_y, 0.5 * y / flap_tip_y)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# first, get displacement independent of x, only dependent on y and t
# maximal displacement at flap tip should be 0.5
# initially, flap's displacement is 0
x_displ = np.minimum(((np.sin(3 * np.pi * t + np.arcsin(-0.95)) + 0.95) / 8) * y / flap_tip_y, 0.5 * y / flap_tip_y)
# first, get displacement independent of x, only dependent on y and t
# initially, flap's displacement is 0
max_x_displacement = 0.5
x_displ = np.minimum(((np.sin(3 * np.pi * t + np.arcsin(-0.95)) + 0.95) / 8) * y / flap_tip_y, max_x_displacement * y / flap_tip_y)

I would generally try to avoid raw numbers and rather use descriptive names. This often allows you to completely save some comments.

What does the 0.95, the 3 * np.pi and the /8 do? Generally, I think if the formula you are using here works we can keep it. But a brief comment about the general rationale behind the arcsin and sin would be helpful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added some comments that hopefully clarify the idea behind the formula

Copy link
Member

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

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

Thank for the changes! Looks good now. Let's get this merged 🎉

@BenjaminRodenberg BenjaminRodenberg merged commit bb17d2c into precice:develop Aug 19, 2024
@NiklasVin NiklasVin deleted the solid-fake branch August 19, 2024 12:42
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.

3 participants