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

Coarse-grained runtime statistics #1067

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Coarse-grained runtime statistics #1067

wants to merge 3 commits into from

Conversation

njansson
Copy link
Collaborator

@njansson njansson commented Dec 8, 2023

Add optional collection of coarse-grained runtime statistics for a simulation.

Enabled via the case file, e.g.

  "runtime_statistics": {
    "enabled": true,
    "start_step": 100,
  },

@njansson njansson added the enhancement New feature or request label Dec 8, 2023
@njansson njansson self-assigned this Dec 8, 2023
call json_get(params, 'case.timestep', dt)
call json_get(params, 'case.end_time', end_time)

this%total_steps_ = ceiling(end_time / dt)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Assumes constant dt, right? We should probably avoid this in new code.


this%total_steps_ = ceiling(end_time / dt)

allocate(this%data(this%total_steps_, size(RT_STATS_ID)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess to avoid constant dt we would need data to a sort of dynamic array which gets expanded on demand. end_time / dt would be a good initial size, I guess.

Comment on lines +51 to +60
logical, private :: enabled_
real(kind=dp), private, allocatable :: elapsed_time_(:)
integer, private, allocatable :: nsamples_(:)
integer, private :: start_step_
integer, private :: total_steps_
real(kind=dp), allocatable :: data(:,:)
contains
procedure, pass(this) :: init => runtime_stats_init
procedure, pass(this) :: record => runtime_stats_record
procedure, pass(this) :: report => runtime_stats_report
Copy link
Collaborator

Choose a reason for hiding this comment

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

Docstring for each component and TBP, please :-). Also, param-doctrings for the TBPs further below.

@@ -120,6 +126,7 @@ subroutine neko_solve(C)
'Elapsed time (s):', end_time-start_time_org, ' Step time:', &
end_time-start_time
call neko_log%end_section(log_buf)
call rt_stats%record(RT_STATS_FLUID, end_time-start_time, t, tstep)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be better to have the objects in question own their own rt_stats and update them? That would clean the main time-loop and allow for more granularity, e.g. one could enable for fluid and disable for scalar. We would only need an extra subroutine to pull in the stuff from all the objects and print the summary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great idea! I'll restructure it

@njansson njansson added the don't merge Don't merge yet! label Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
don't merge Don't merge yet! enhancement New feature or request
Projects
No open projects
Status: 🙅Skipped
Development

Successfully merging this pull request may close these issues.

None yet

2 participants