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

Use output dates from atmos.integrator #725

Closed
wants to merge 2 commits into from
Closed

Conversation

Sbozzolo
Copy link
Member

@Sbozzolo Sbozzolo commented Apr 4, 2024

This PR ensures that the leaderboard uses the output dates from atmos.

@Sbozzolo Sbozzolo marked this pull request as ready for review April 7, 2024 01:59
@Sbozzolo Sbozzolo requested a review from LenkaNovak April 7, 2024 02:00
Copy link
Collaborator

@LenkaNovak LenkaNovak left a comment

Choose a reason for hiding this comment

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

LGTM, thank you, @Sbozzolo! Just testing if two seasons produce different outputs here. Once checked, feel free to merge.

@@ -858,20 +858,22 @@ if ClimaComms.iamroot(comms_ctx)
# Compare against observations
if t_end > 84600
@info "Error against observations"
output_dates = cs.dates.date0[] .+ Second.(atmos_sim.integrator.sol.t)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good to add a comment that dt_save_to_sol needs to be consistent with the required averaging. For most of our runs dt_save_to_sol is 10days but the seasonal average using this method would require 1days (for daily averaged data).

@LenkaNovak
Copy link
Collaborator

Confirmed that MAM and DJF differ. 👍

@LenkaNovak LenkaNovak mentioned this pull request Apr 9, 2024
12 tasks
@Sbozzolo Sbozzolo enabled auto-merge April 9, 2024 17:08
@juliasloan25
Copy link
Member

@Sbozzolo can this be merged?

@Sbozzolo
Copy link
Member Author

Was cherry picked in 79594b1

@Sbozzolo Sbozzolo closed this Apr 19, 2024
auto-merge was automatically disabled April 19, 2024 00:58

Pull request was closed

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