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

Addition of capability to do force calculation using 3D CV method #269

Merged
merged 12 commits into from
Jun 17, 2024

Conversation

airwarriorg91
Copy link
Contributor

Hii,
I have implemented all the recommended changes in the previous PR. I have tried to keep the changes to minimum. I have also added a checking of errors after each mpi call. Instead of the creating new variables, I have used MPI_IN_PLACE. Kindly go through the changes and let me know if any further modification is required. I have tested the code for both Cylinder and Sphere at Re=3700 case. It is working fine.

I have also created a new input file under the cylinder-wake case to represent the usage of new parameters (zfr, zbk, i2dsim, cez). By default the value of i2dsim=1 i.e. the usual 2D CV method is used. If changed to 1, it used the new 3D CV method.

Also I noticed an inconsitency in the formatting of the code, might be fixed in the later versions in some lines 2 spaces (Example of 2 space) indentation is being used whereas somewhere 3 spaces indentation is used (Example of 3 space).

Thanks,
Gaurav

Copy link
Contributor

@mathrack mathrack left a comment

Choose a reason for hiding this comment

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

Hello,

Thank you for your PR. The formatting issue will be solved at the scale of the full code later using an automated script.

I have suggested some modifications. Please look at them.

Cheers

src/forces.f90 Outdated Show resolved Hide resolved
src/forces.f90 Outdated Show resolved Hide resolved
src/forces.f90 Outdated Show resolved Hide resolved
src/forces.f90 Show resolved Hide resolved
src/forces.f90 Show resolved Hide resolved
src/forces.f90 Show resolved Hide resolved
src/forces.f90 Show resolved Hide resolved
src/forces.f90 Outdated Show resolved Hide resolved
src/forces.f90 Outdated Show resolved Hide resolved
src/forces.f90 Outdated Show resolved Hide resolved
@mathrack
Copy link
Contributor

mathrack commented May 14, 2024

The objective of the PR is to perform 2D force calculation on a 3D object ?

Do we recover the 2D force when zfr(1) = -999999.0 and zbk(1) = 999999.0 ?

@airwarriorg91
Copy link
Contributor Author

The objective of the PR is to perform 2D force calculation on a 3D object ?

Do we recover the 2D force when zfr(1) = -999999.0 and zbk(1) = 999999.0 ?

No, the objective is to perform force calculation for objects like Sphere or any other 3D body which is not span-wise symmetric like a Cylinder, or Airfoil.

@airwarriorg91
Copy link
Contributor Author

Hello,

Thank you for your PR. The formatting issue will be solved at the scale of the full code later using an automated script.

I have suggested some modifications. Please look at them.

Cheers

Cool,
I had tried to keep the same format for derivatives as it is in the Case-Cylinder-Wake. Using different names for derivatives might not be a good idea. That's why I changed the names.

I can shift the extra derivatives and operations inside the i3dsim==0 case as it is not required and affect the code performance unnecessarily.

Thanks,
Gaurav

@mathrack
Copy link
Contributor

The objective of the PR is to perform 2D force calculation on a 3D object ?
Do we recover the 2D force when zfr(1) = -999999.0 and zbk(1) = 999999.0 ?

No, the objective is to perform force calculation for objects like Sphere or any other 3D body which is not span-wise symmetric like a Cylinder, or Airfoil.

Please elaborate more clearly the objective of the PR. It looks like the output is a 2D force, whatever the value of i2dsim.

When the 3D force calculation is used on a 2D object with zfr(1) = -999999.0 and zbk(1) = 999999.0, do we recover the 2D force calculation ?

@airwarriorg91
Copy link
Contributor Author

The objective of the PR is to perform 2D force calculation on a 3D object ?
Do we recover the 2D force when zfr(1) = -999999.0 and zbk(1) = 999999.0 ?

No, the objective is to perform force calculation for objects like Sphere or any other 3D body which is not span-wise symmetric like a Cylinder, or Airfoil.

Please elaborate more clearly the objective of the PR. It looks like the output is a 2D force, whatever the value of i2dsim.

When the 3D force calculation is used on a 2D object with zfr(1) = -999999.0 and zbk(1) = 999999.0, do we recover the 2D force calculation ?

The objective of the PR is to calculate Lift and Drag for 3D objects like a sphere or something which is not extruded in the the Z direction.

The current 2d version only does calculation for objects extruded in Z direction. Refer to discussion #256 for more information. (#256)

If we set the zfr(1) = 0 and zbk(1)=Lz for a cylinder case, we will recover the same value of the drag ignoring effects due the vortex breakdown in the 3D case. If you run the calculation for a sphere, you will get different drag and lift values and the 3D one will give you the correct value.

I hope it is clear.

@airwarriorg91
Copy link
Contributor Author

Hello,
I have made the suggested changes in the PR.

  1. When i2dsim==1, the extra derv and transpose functions are called.
  2. When i2dsim==1, modified format is printed in the log.
  3. The checkpoint save operation is done outside the do loop as well as if else block once. The operation isn't required to be called for each CV as well doesn't depend on the type of method.
  4. Error code is checked after every MPI operation and MPI_ABORT was changed with DECOMP_2D_ABORT.

For the naming of the derivatives, I have kept the variable names same as that in the Case-Cylinder-wake.f90. It avoids confusion for further updates.

Thanks,
Gaurav

@mathrack
Copy link
Contributor

Thank you. It looks ok. I need a few days to check it.

@mathrack
Copy link
Contributor

The current version of the PR is looking ok from my perspective. I think we could avoid duplicate and have some force calculation shared between the 2D and 3D parts. But this goes beyond the scope of the PR.

I have not yet tested the force calculation, I will try to do that during the next days. I have made some modifications in the PR. Thus it is probably best to wait for comments from another developer before merging.

@airwarriorg91
Copy link
Contributor Author

airwarriorg91 commented May 30, 2024

The current version of the PR is looking ok from my perspective. I think we could avoid duplicate and have some force calculation shared between the 2D and 3D parts. But this goes beyond the scope of the PR.

I have not yet tested the force calculation, I will try to do that during the next days. I have made some modifications in the PR. Thus it is probably best to wait for comments from another developer before merging.

Hii,
The changes look good to me. I had checked the code on my local system as well as on HPC before your commits. It worked fine on both of them. Please check it once and let me know if any changes are required.

I also wanted to discuss a few more stuff about xcompact in general (not related to the PR).

Thanks,
Gaurav

The non-linear term should not be estimated with the interpolated velocity : (a+b)² /= a² + b²
Copy link
Collaborator

@rfj82982 rfj82982 left a comment

Choose a reason for hiding this comment

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

LGTM

@rfj82982 rfj82982 merged commit 4ea725e into xcompact3d:master Jun 17, 2024
1 check passed
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.

None yet

3 participants