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

Background Fields: Fix Restart GUARD #2139

Merged

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Jul 17, 2017

(Trying) to close #2137:
Restart of background fields could cause:

  • artificial noise after restart from the outer regions with non-periodic boundary conditions
  • if background fields are used (e.g. TWTS pulse)

Simplify the logic of restarting fields:

  • do not add the already contained fields in movingWindowCheck again
  • spares us the substraction we had before
  • GUARDS are not part of the checkpoint data, which is cumbersome for the "outer" GUARDS of our global domain
  • we need to add the background field ONLY in those to generate a valid restart field

@ax3l ax3l added bug a bug in the project's code component: plugin in PIConGPU plugin labels Jul 17, 2017
@ax3l ax3l added this to the 0.4.0 / 1.0.0: Next Stable milestone Jul 17, 2017
PrometheusPi
PrometheusPi previously approved these changes Jul 17, 2017
Copy link
Member

@PrometheusPi PrometheusPi left a comment

Choose a reason for hiding this comment

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

Looks good to me - but let us perform a run time test.

@ax3l ax3l force-pushed the fix-RestartBGFields branch 2 times, most recently from f3d1c21 to fd795b0 Compare July 17, 2017 13:38
Simplify the logic of restarting fields:
- do not add the already contained fields in
  `movingWindowCheck` again
- spares us the substraction we had before
- GUARDS are not part of the checkpoint data,
  which is cumbersome for the "outer" GUARDS
  of our global domain
- we need to add the background field ONLY in those
  to generate a valid restart field
@ax3l ax3l force-pushed the fix-RestartBGFields branch from fd795b0 to 494865e Compare July 17, 2017 13:39
@psychocoderHPC
Copy link
Member

I verified this fix with the field background example from #2137. I used only the field background for the E field.

PrometheusPi pushed a commit to PrometheusPi/picongpu that referenced this pull request Jul 17, 2017
@PrometheusPi
Copy link
Member

@ax3l l see above for the backport to 0.3.0 - it is the last commit

@PrometheusPi
Copy link
Member

run time test results:
fixed
(for the 0.3.0 backport)

PrometheusPi pushed a commit to PrometheusPi/picongpu that referenced this pull request Jul 17, 2017
@ax3l
Copy link
Member Author

ax3l commented Jul 17, 2017

@BeyondEspresso please take note of this bugfix fo restarts with background fields if you have not seen it yet. also applies to (all) previous releases! (sorry :( ) included since we added the field background to the plugins (for you ;) )

@BeyondEspresso
Copy link
Member

Guys, this is awesome! 😃 Great work, all of you! 🥇

@psychocoderHPC psychocoderHPC merged commit 147ea91 into ComputationalRadiationPhysics:dev Jul 18, 2017
@psychocoderHPC psychocoderHPC self-assigned this Jul 18, 2017
@ax3l ax3l deleted the fix-RestartBGFields branch July 18, 2017 07:55
ax3l added a commit to ax3l/picongpu that referenced this pull request Jul 18, 2017
@ax3l ax3l mentioned this pull request Jul 18, 2017
1 task
@ax3l ax3l added the affects latest release a bug that affects the latest stable release label Jul 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects latest release a bug that affects the latest stable release bug a bug in the project's code component: plugin in PIConGPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants