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

CI: fix fluctuations in collisionXZ checksums #5132

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

EZoni
Copy link
Member

@EZoni EZoni commented Aug 13, 2024

Trying to see if we can get under control the fluctuations observed in the checksums of the collisionXZ test.

@EZoni EZoni added the component: tests Tests and CI label Aug 13, 2024
@EZoni EZoni changed the title CI: reset collisionXZ checksums [WIP] CI: reset collisionXZ checksums Aug 13, 2024
@EZoni EZoni changed the title [WIP] CI: reset collisionXZ checksums [WIP] CI: fix fluctuations in collisionXZ test Aug 13, 2024
@EZoni EZoni force-pushed the EZoni_ci_fix_collisionXZ branch 2 times, most recently from 6509b87 to fe3b813 Compare August 13, 2024 16:58
@EZoni EZoni changed the title [WIP] CI: fix fluctuations in collisionXZ test [WIP] CI: fix fluctuations in collisionXZ checksums Aug 13, 2024
@EZoni EZoni added bug Something isn't working bug: affects latest release Bug also exists in latest release version component: collisions Anything related to particle collisions labels Aug 13, 2024
@ax3l
Copy link
Member

ax3l commented Aug 13, 2024

That parameter should be set here already:

'warpx.do_dynamic_scheduling=0 warpx.serialize_initial_conditions=1',

@EZoni
Copy link
Member Author

EZoni commented Aug 14, 2024

That parameter should be set here already:

'warpx.do_dynamic_scheduling=0 warpx.serialize_initial_conditions=1',

That's right, thanks for pointing it out. I always forget that we set input parameters in three different places at the moment. For now I'm just keeping this PR open (now it simply increases the relative tolerance for the checksums) until the next meeting where this issue will be discussed.

@ax3l ax3l changed the title [WIP] CI: fix fluctuations in collisionXZ checksums CI: fix fluctuations in collisionXZ checksums Aug 14, 2024
@ax3l
Copy link
Member

ax3l commented Aug 14, 2024

Since development is failing, can you set a tolerance just high enough for it to pass and we merge it?

@EZoni
Copy link
Member Author

EZoni commented Aug 14, 2024

I'm afraid 1e-1 might be the one that is just high enough. At least on my computer, the test fails with 1e-2. I think this is consistent with the "10%" error that @RemiLehe mentioned offline. I could check if this is the case also on Azure, but it is for sure on my computer. This holds for both collisionXZ and collisionXYZ.

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.

OK for me for now.

@RemiLehe @JustinRayAngus what do you think, shall we use this for now?

Copy link
Member

@roelof-groenewald roelof-groenewald left a comment

Choose a reason for hiding this comment

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

I agree this is good to do until the underlying issue with the collision tests can be sorted out.

@RemiLehe RemiLehe merged commit a58ddc0 into ECP-WarpX:development Aug 14, 2024
47 checks passed
@EZoni EZoni deleted the EZoni_ci_fix_collisionXZ branch August 14, 2024 17:17
Copy link
Contributor

@JustinRayAngus JustinRayAngus left a comment

Choose a reason for hiding this comment

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

Approve. What's different between these two tests and the other tests that use Coulomb collisions?

@ax3l
Copy link
Member

ax3l commented Aug 15, 2024

We don't understand that yet... Maybe less particles per cell or per simulation and thus worse statistics?

@ax3l
Copy link
Member

ax3l commented Aug 15, 2024

In #5126 I see that collisionZ fluctuates a well.

./analysis_collision_1d.py  collisionZ_plt000600
TApar at 30ps error =  0.0025701770198334937
tolerance =  0.02
ERROR: Benchmark and output file checksum have different value for key [ions,particle_momentum_x]
Benchmark: [ions,particle_momentum_x] 3.425400072687143e-16
Test file: [ions,particle_momentum_x] 3.424633029669351e-16
Absolute error: 7.67e-20
Relative error: 2.24e-04
ERROR: Benchmark and output file checksum have different value for key [ions,particle_momentum_y]
Benchmark: [ions,particle_momentum_y] 3.421937133999805e-16
Test file: [ions,particle_momentum_y] 3.429026824477811e-16
Absolute error: 7.09e-19
Relative error: 2.07e-03
ERROR: Benchmark and output file checksum have different value for key [ions,particle_momentum_z]
Benchmark: [ions,particle_momentum_z] 5.522701882677923e-16
Test file: [ions,particle_momentum_z] 5.528396735566589e-16
Absolute error: 5.69e-19
Relative error: 1.03e-03
ERROR: Benchmark and output file checksum have different value for key [ions,particle_position_x]
Benchmark: [ions,particle_position_x] 7.200011611411148e+02
Test file: [ions,particle_position_x] 7.200708684755696e+02
Absolute error: 6.97e-02
Relative error: 9.68e-05

Notable differences in runners:

  • Azure Pipelines 9 (newer) in PR vs. Azure Pipelines 8 on development
  • CPUs: Xeon(R) Platinum 8272CL "Cascade Lake" (~2019) on PR vs. Xeon(R) CPU E5-2673 v4 "Broadwell" (~2016) on development

ax3l added a commit to ax3l/WarpX that referenced this pull request Aug 16, 2024
@ax3l ax3l mentioned this pull request Aug 16, 2024
ax3l added a commit to ax3l/WarpX that referenced this pull request Aug 16, 2024
EZoni pushed a commit that referenced this pull request Aug 16, 2024
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: collisions Anything related to particle collisions component: tests Tests and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants