Skip to content

Add verbose message to assert when using adaptive time step#1321

Merged
MaxThevenet merged 1 commit into
Hi-PACE:developmentfrom
titoiride:verbose_adaptive_time_step
Dec 2, 2025
Merged

Add verbose message to assert when using adaptive time step#1321
MaxThevenet merged 1 commit into
Hi-PACE:developmentfrom
titoiride:verbose_adaptive_time_step

Conversation

@titoiride
Copy link
Copy Markdown
Contributor

Happy to discard if this is incorrect, but it seems to me that the point of this assert is to avoid using the adaptive time step if laser pulses are present.
I tried doing that before and the code failed on the assert, but there was not any useful message.
If this is a general limitation, it's probably better to warn the user with a clear message.

  • Small enough (< few 100s of lines), otherwise it should probably be split into smaller PRs
  • Tested (describe the tests in the PR description)
  • Runs on GPU (basic: the code compiles and run well with the new module)
  • Contains an automated test (checksum and/or comparison with theory)
  • Documented: all elements (classes and their members, functions, namespaces, etc.) are documented
  • Constified (All that can be const is const)
  • Code is clean (no unwanted comments, )
  • Style and code conventions are respected at the bottom of https://github.com/Hi-PACE/hipace
  • Proper label and GitHub project, if applicable

Copy link
Copy Markdown
Member

@AlexanderSinn AlexanderSinn left a comment

Choose a reason for hiding this comment

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

Indeed, the laser solver assumes a constant dt between the three time steps that it uses. So it is not possible to use it with an adaptive (non-constant) time step.  

@MaxThevenet
Copy link
Copy Markdown
Member

You're correct, thanks for the fix!

@MaxThevenet MaxThevenet merged commit 3905e43 into Hi-PACE:development Dec 2, 2025
9 of 10 checks passed
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.

3 participants