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 PICMI Scripts for LWFA Tests #2700

Merged
merged 24 commits into from
Jan 3, 2022
Merged

Conversation

EZoni
Copy link
Member

@EZoni EZoni commented Dec 21, 2021

Add PICMI scripts and CI tests for LWFA in 1D, 2D, 3D and RZ geometries, mimicking the original input files in Examples/Physics_applications/laser_acceleration/.

We can probably leave the following three items to separate follow-up PRs:

  • 2D w/ boosted frame
  • Add electron beam to all tests
  • Set same warpx.cfl in all tests

@EZoni EZoni added component: tests Tests and CI component: PICMI Standardized input format labels Dec 21, 2021
@EZoni
Copy link
Member Author

EZoni commented Dec 21, 2021

@ax3l Did you want me to add these as CI tests as well? I'm not sure I understood that correctly. If not, I will have to rename the PICMI scripts avoiding the PICMI_inputs name tag, because we have a style check which requires all files in the Examples/ directory that start with inputs or PICMI_inputs to be tested automatically.

@EZoni EZoni marked this pull request as ready for review December 21, 2021 23:19
@EZoni EZoni requested review from dpgrote and ax3l December 21, 2021 23:30
@dpgrote
Copy link
Member

dpgrote commented Dec 22, 2021

There is already the test case PICMI_inputs_laser_acceleration.py. Is there anything different in what is being tested between it and the new 3d test case? Perhaps PICMI_inputs_laser_acceleration.py can be deleted?

@EZoni
Copy link
Member Author

EZoni commented Dec 22, 2021

@dpgrote Thank you, that's a good point. I think we can remove that script, because it doesn't reflect 100% the corresponding input file inputs_3d, which I understood was among the aims of having this new set of scripts. Let's see what @ax3l says. Anyways, I'll add a point to the PR description, to make sure that we don't forget.

@ax3l
Copy link
Member

ax3l commented Dec 22, 2021

Yes, a perfect copy of the inputs_... scripts in PICMI would be fantastic.

Feel free to remove the other one to avoid confusing as Dave suggests.

yes, please add the new PICMI inputs to Warpx-tests.ini so they are covered/do not run out-of-date.

@ax3l
Copy link
Member

ax3l commented Dec 22, 2021

warpx.cfl is set to 0.9 in the original 1D input file and to 1.0 in the original 2D, 3D and RZ input files. Do we want to set warpx.cfl = 0.9 (or some other values between 0.9 and 1.0) for all cases instead?

Let's fix that in the 1D case. 0.9 is an old value, we should not suggest that to users.
cc @prkkumar

@ax3l
Copy link
Member

ax3l commented Dec 22, 2021

The electron beam is missing from the original 1D and 3D input files, hence it's NOT been added to the corresponding PICMI scripts either. Do we want to have an electron beam also in the 1D and 3D tests instead?

I would actually say we should add a witness beam in all those inputs. makes for a nice test and vis case as well.

This could replace #1631 then.

@ax3l
Copy link
Member

ax3l commented Dec 22, 2021

Thanks a lot for adding these, @EZoni !! 🙏

@@ -1657,24 +1657,6 @@ compareParticles = 1
particleTypes = electrons beam
analysisRoutine = Examples/analysis_default_regression.py

[Python_LaserAccelerationMR]
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was associated with the old PICMI script that was removed, and I think it was not using mesh refinement (despite what the name tag MR may suggest).

@EZoni
Copy link
Member Author

EZoni commented Dec 22, 2021

I suggest that we push any changes that affect the benchmarks (e.g. CFL value or adding a witness beam) in a separate PR. Then, this PR should not require any benchmark reset. Moreover, the new benchmarks for the new Python tests will be the exact copy of the existing ones, if the PICMI scripts mimic the corresponding original input files correctly.

@@ -1,25 +1,46 @@
{
"beam": {
Copy link
Member Author

Choose a reason for hiding this comment

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

Keep in mind that this is actually a new, different test, but shows up as a benchmark change because an old test with the same name (but actually different setup) was removed (see other comment on WarpX-tests.ini).

@EZoni
Copy link
Member Author

EZoni commented Dec 23, 2021

Note that the changes in

  • .github/workflows/intel.yml
  • .github/workflows/macos.yml
  • .github/workflows/ubuntu.yml

where I simply replaced the old PICMI script PICMI_inputs_laser_acceleration.py with the new PICMI script PICMI_inputs_3d.py (also tested successfully in CI), cause some failures in the macOS and Intel builds. Have not understood why yet.

@EZoni EZoni changed the title [WIP] Add PICMI Scripts for LWFA Tests Add PICMI Scripts for LWFA Tests Dec 23, 2021
@ax3l
Copy link
Member

ax3l commented Jan 3, 2022

I merged development now (to incorporate #2556) and I fixed the three mentioned tests (same as #2673): they lacked the first line to declare the python interpreter :)

Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Thank you :)

@ax3l ax3l merged commit ce5914f into ECP-WarpX:development Jan 3, 2022
@EZoni EZoni deleted the lwfa_picmi branch January 5, 2022 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: PICMI Standardized input format component: tests Tests and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants