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

Close #1688 : Transform the moving window velocity to its boosted-frame value. #1700

Merged
merged 2 commits into from
Feb 12, 2021

Conversation

steelan
Copy link
Contributor

@steelan steelan commented Feb 10, 2021

Source/Utils/WarpXMovingWindow.cpp : Transform moving_window_v to that in the boosted frame (moving_window_v-beta_boost*c)/(1-moving_window_v*beta_boost/c).

Examples/Tests/moving_window_boost : A simple test gives the moving window velocity in the boosted frame.

…hat in the boosted frame '(moving_window_v-beta_boost*c)/(1-moving_window_v/c*beta_boost)'.

Examples/Tests/moving_window_boost : A simple test gives the moving window velocity in the boosted frame.
@ax3l
Copy link
Member

ax3l commented Feb 10, 2021

Hi @steelan, thank you for your first contribution to WarpX!

We will assign a reviewer from our team to take a look and iterate with you on your changes.

Can you in the meantime please add a detailed pull request description, outlining what you try to achieve and how?
Update: Oh, is this related to #1688? Then just linking this in the PR description is clear enough :)

Thank you already and again welcome ✨

@ax3l ax3l added the component: core Core WarpX functionality label Feb 10, 2021
@ax3l ax3l requested a review from RemiLehe February 10, 2021 21:02
z2max=ds.domain_right_edge.v[2]

vv=(z2max-z1max)/(t2-t1)/299792458.
print('moving_window_v in boosted frame =', vv)
Copy link
Member

Choose a reason for hiding this comment

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

I think we could write an additional line here, comparing the value in vv with an expected value (within an epsilon range) and returning a non-zero exit code with sys.exit(1) in case the value does not match.

That way, we can verify the test without manual intervention.

@RemiLehe RemiLehe changed the title Source/Utils/WarpXMovingWindow.cpp : Transform 'moving_window_v' to t… Close #1688 : Transform the moving window velocity to its boosted-frame value. Feb 10, 2021
Copy link
Member

@RemiLehe RemiLehe left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR!
The changes to WarpX itself look good. Could you add some more explanations in the file that defines the automated test? Thanks!

@@ -0,0 +1,22 @@
import os
import yt
Copy link
Member

Choose a reason for hiding this comment

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

Could add a few lines of comments here, in order to explain what the tests does, and what it is checking at the end of the simulation?

@RemiLehe
Copy link
Member

After more discussion within the WarpX team, we think that an automated test is not necessarily needed for this feature.
Since @steelan tested this by hand, I think that we can merge as is.

@steelan
Copy link
Contributor Author

steelan commented Feb 11, 2021 via email

@steelan
Copy link
Contributor Author

steelan commented Feb 11, 2021 via email

@RemiLehe RemiLehe merged commit 2023486 into ECP-WarpX:development Feb 12, 2021
@ax3l ax3l added bug Something isn't working bug: affects latest release Bug also exists in latest release version labels Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: affects latest release Bug also exists in latest release version bug Something isn't working component: core Core WarpX functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants