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

Fix bug in forces.f90 and enable force calculation for sphere example. #281

Closed
wants to merge 0 commits into from

Conversation

airwarriorg91
Copy link
Contributor

Hello,
There is a small bug in the force.f90 file

xDrag_mean = xmom + convx - pressx -stressx
and
yLift_mean = ymom + convy - pressy -stressy

The negative sign before xmom and ymom was missing.

Also, I have made modifications to enable force calcualtion in the sandbox mode if iforces=1 for immersed bodes using the subroutines in forces.f90. There are two separate .i3d files in examples/Sphere. The file gen.i3d is used to generate the necessary files for sandbox mode and the input.i3d is used for actual simulation. As xcompact3d-toolbox doesn't recognize the parameters iforces and other force calculation parameters.

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.

The description of the PR claims there is a bug in the force.f90 file. But the PR is not changing the file force.f90.

This PR should focus on fixing the bug. Please keep the PR small and dedicated to one clear objective. Please open a dedicated PR to enable force calculation for sphere example.

The previous PR updating the force calculation was accepted but it contained a bug. Please provide results showing the current code is broken and will be fixed thanks to the present PR.

@@ -275,7 +275,7 @@ subroutine init_xcompact3d()
open(42,file='time_evol.dat',form='formatted')
endif
endif
if (itype==5) then
if (iforces==1) then
Copy link
Contributor

Choose a reason for hiding this comment

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

The statement used to close the file unit 38 should also be modified. See https://github.com/xcompact3d/Incompact3d/blob/master/src/xcompact3d.f90#L312

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello,
Sorry for mixing two changes in one PR. I will open a dedicated PR for the sphere example. Also regarding the bug in the force.f90 file, if you refer to my original commit in the PR #269, there was minus sign infront of the xmom and ymom terms which got missing after the review commits from your side. Can you please check once again ? I have been using the my local version instead of the latest commit all this time, so didn't realize it. But few days back, I tried it and got negative drag and lift coeffficient for the sphere problem due to the missing minus sign.

Thanks,
Gaurav

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now I will retract commits which retract all the commits except the forces-bug-fix. After the merging of this PR will open another PR for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello, Sorry for mixing two changes in one PR. I will open a dedicated PR for the sphere example. Also regarding the bug in the force.f90 file, if you refer to my original commit in the PR #269, there was minus sign infront of the xmom and ymom terms which got missing after the review commits from your side. Can you please check once again ? I have been using the my local version instead of the latest commit all this time, so didn't realize it. But few days back, I tried it and got negative drag and lift coeffficient for the sphere problem due to the missing minus sign.

Thanks, Gaurav

Hello, I am not sure to understand what you are talking about. The commit history of the code is public. Feel free to point to a specific modification in a specific commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I had mistaken about the minus sign being missing in the previous commits. I will revert all the commits and make a new commit with the bug fixed and results. Sorry for the confusion ! :)

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

2 participants