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

Add Functions::RayleighKotheVortex #15932

Merged
merged 1 commit into from Sep 4, 2023

Conversation

peterrum
Copy link
Member

No description provided.

@peterrum peterrum requested a review from blaisb August 26, 2023 20:34
Copy link
Member

@blaisb blaisb left a comment

Choose a reason for hiding this comment

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

Two small comments, nothing big


const double px = numbers::PI * point(0);
const double py = numbers::PI * point(1);
const double pt = numbers::PI / T * t;
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if that's the best variable name. pt for me mentally translates to point... but maybe that's just me :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I am fine with the names.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe

Suggested change
const double pt = numbers::PI / T * t;
const double pi_t = numbers::PI / T * t;

and similarly above?

Copy link
Member Author

Choose a reason for hiding this comment

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

@masterleinad I went with your name suggestion. Thanks 👍

@@ -278,7 +236,7 @@ namespace Step68
MappingQ1<dim> mapping;
LinearAlgebra::distributed::Vector<double> velocity_field;

Vortex<dim> velocity;
Functions::RayleighKotheVortex<dim> velocity;
Copy link
Member

Choose a reason for hiding this comment

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

Can you also make the same change in the performance benchmark while we are at it?
:)

@peterrum
Copy link
Member Author

/rebuild

/**
* A class that represents a time-dependent function object for a
* Rayleigh--Kothe vortex vector field. This is generally used as
* flow pattern in complex test cases for interface tracking methods
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
* flow pattern in complex test cases for interface tracking methods
* as flow pattern in complex test cases for interface tracking methods

?

Copy link
Member Author

Choose a reason for hiding this comment

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

The line above contains the as.


const double px = numbers::PI * point(0);
const double py = numbers::PI * point(1);
const double pt = numbers::PI / T * t;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe

Suggested change
const double pt = numbers::PI / T * t;
const double pi_t = numbers::PI / T * t;

and similarly above?

@peterrum peterrum merged commit b4da721 into dealii:master Sep 4, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants