Skip to content

Only print phase control phase change if phase is actually different #4229

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.

Sign up for GitHub

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

Jump to bottom
Merged
wthrowe merged 1 commit into from
Sep 16, 2022
Merged

Only print phase control phase change if phase is actually different #4229

wthrowe merged 1 commit into from
Sep 16, 2022

Conversation

@knelli2
Copy link
Contributor

@knelli2 knelli2 commented Sep 15, 2022 •
edited
Loading

Proposed changes

This is to avoid lots of output whenever the phase change trigger is checked often. Previously in release mode, my output file looks like:

ControlSystemAhA: t=0: its=27: -7.0e-07<R<9e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhB: t=0: its=27: -7.1e-07<R<9e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
Entering phase: Evolve
ControlSystemAhA: t=0.015: its=9: -6.5e-07<R<8e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhB: t=0.015: its=9: -6.5e-07<R<8e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868

Now, it'll only print if the phase control actually changed phases.

Upgrade instructions

Code review checklist

  • The code is documented and the documentation renders correctly. Run
    make doc to generate the documentation locally into BUILD_DIR/docs/html.
    Then open index.html.
  • The code follows the stylistic and code quality guidelines listed in the
    code review guide.
  • The PR lists upgrade instructions and is labeled bugfix or
    new feature if appropriate.

Further comments

@wthrowe
Copy link
Member

wthrowe commented Sep 15, 2022

How hard would it be to check whether the phase actually changed, and only print in that case?

@knelli2
Copy link
Contributor Author

knelli2 commented Sep 15, 2022

How about

if (next_phase.has_value() and current_phase_ != next_phase.value()) {
    current_phase_ = next_phase.value();
    Parallel::printf("Entering phase from phase control: %s\n", next_phase.value());
}

@wthrowe
Copy link
Member

wthrowe commented Sep 15, 2022

Sure, that looks simple enough. If you change the existing if statement make sure you don't break the else branch.

@knelli2 knelli2 force-pushed the phase_control_print_debug branch from aae2f4e to 8b67fdb Compare September 15, 2022 23:36
@knelli2
Copy link
Contributor Author

knelli2 commented Sep 15, 2022

Opted for a simple nested if. Now the output of a BBH simulation in Release mode which has a phase change trigger being checked every step (but only run once) looks like this:

Entering phase: RegisterWithElementDataReader
Entering phase: ImportInitialData
Entering phase: InitializeInitialDataDependentQuantities
Entering phase: Register
Entering phase: InitializeTimeStepperHistory
Entering phase: Evolve
ControlSystemAhA: t=0: its=27: -7.0e-07<R<9e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhB: t=0: its=27: -7.1e-07<R<9e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhA: t=0.015: its=9: -6.5e-07<R<8e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhB: t=0.015: its=9: -6.5e-07<R<8e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhA: t=0.03: its=9: -6.1e-07<R<7e-07, |R|=1e-09, |R_grid|=1e-07, 0.4835<r<0.4868
ControlSystemAhB: t=0.03: its=9: -6.2e-07<R<7e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhA: t=0.045: its=12: -5.9e-07<R<7e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhB: t=0.045: its=12: -5.9e-07<R<7e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhA: t=0.06: its=13: -5.7e-07<R<7e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
ControlSystemAhB: t=0.06: its=13: -5.8e-07<R<7e-07, |R|=1e-09, |R_grid|=2e-07, 0.4835<r<0.4868
Entering phase from phase control: WriteCheckpoint
[0] Checkpoint starting in SpectreCheckpoint000000
Checkpoint to disk finished in 2.677493s, sending out the cb...
Entering phase from phase control: Exit

Done!
Wall time in seconds: 182.807973
Date and time at completion: Thu Sep 15 16:30:25 2022

@knelli2 knelli2 force-pushed the phase_control_print_debug branch from 8b67fdb to 53b3f81 Compare September 15, 2022 23:39
@knelli2 knelli2 changed the title Only print phase control phase change in debug mode Only print phase control phase change if phase is actually different Sep 15, 2022
@wthrowe wthrowe merged commit 7ff58fe into sxs-collaboration:develop Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wthrowe wthrowe wthrowe approved these changes

@nilsdeppe nilsdeppe nilsdeppe approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@knelli2 @wthrowe @nilsdeppe