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

Do not exercise diagnostics when disabled #3038

Merged
merged 1 commit into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config/default_configs/default_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ override_τ_precip:
output_default_diagnostics:
help: "Output the default diagnostics associated to the selected atmospheric model"
value: true
enable_diagnostics:
help: "Set to false to fully disable the diagnostics"
value: true
charleskawczynski marked this conversation as resolved.
Show resolved Hide resolved
netcdf_output_at_levels:
help: "Do not perform any vertical interpolation in the output NetCDF files. Instead, interpolate horizontally and output the level. "
value: true
Expand Down
2 changes: 1 addition & 1 deletion src/solver/solve.jl
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function solve_atmos!(simulation)
maxrss_str = prettymemory(maxrss())
@info "Memory currently used (after solve!) by the process (RSS): $maxrss_str"

foreach(close, output_writers)
isnothing(output_writers) || foreach(close, output_writers)
end
end

Expand Down
38 changes: 22 additions & 16 deletions src/solver/type_getters.jl
Original file line number Diff line number Diff line change
Expand Up @@ -663,17 +663,21 @@ function get_simulation(config::AtmosConfig)
@info "get_callbacks: $s"

# Initialize diagnostics
s = @timed_str begin
scheduled_diagnostics, writers = get_diagnostics(
config.parsed_args,
atmos,
Y,
p,
t_start,
sim_info.dt,
)
if config.parsed_args["enable_diagnostics"]
charleskawczynski marked this conversation as resolved.
Show resolved Hide resolved
s = @timed_str begin
scheduled_diagnostics, writers = get_diagnostics(
config.parsed_args,
atmos,
Y,
p,
t_start,
sim_info.dt,
)
end
@info "initializing diagnostics: $s"
else
writers = nothing
charleskawczynski marked this conversation as resolved.
Show resolved Hide resolved
end
@info "initializing diagnostics: $s"

continuous_callbacks = tuple()
discrete_callbacks = callback
Expand Down Expand Up @@ -705,13 +709,15 @@ function get_simulation(config::AtmosConfig)
end
@info "init integrator: $s"

s = @timed_str begin
integrator = ClimaDiagnostics.IntegratorWithDiagnostics(
integrator,
scheduled_diagnostics,
)
if config.parsed_args["enable_diagnostics"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if config.parsed_args["enable_diagnostics"]
if !isnothing(scheduled_diagnostics)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to exercise this code, that is the point of this PR.

Copy link
Member

@Sbozzolo Sbozzolo May 22, 2024

Choose a reason for hiding this comment

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

I don't understand. If you add the suggestion above (in the other code chunk), this code is not exercised when there are no diagnostics

Copy link
Member

Choose a reason for hiding this comment

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

So, if you want to not exercise this code, you can just comment out the diagnostics from your configuration

s = @timed_str begin
integrator = ClimaDiagnostics.IntegratorWithDiagnostics(
integrator,
scheduled_diagnostics,
)
end
@info "Added diagnostics: $s"
end
@info "Added diagnostics: $s"

reset_graceful_exit(output_dir)

Expand Down
Loading