feat: expose DateTime clock parameter for CMIP simulations (#1819)

[kavyakapoor420]

Purpose

Replace the default Float64-based clock with a DateTime-based clock in ocean and sea ice simulations to improve temporal precision for CMIP runs.
Closes #1819

To-do

• Add DateTime-based clock support to ocean and sea ice simulations
• Update time-stepping logic to use DateTime arithmetic
• Update docstrings
• Run tests and formatting checks

Content

• Added start_date::DateTime to:

  • OceananigansSimulation
  • ClimaSeaIceSimulation

• Updated constructors to initialize clocks using:
  OC.Clock{DateTime}(time = start_date)

• Replaced Float64-based time stepping with DateTime-based Δt computation in:

  • Interfacer.step! for ocean
  • Interfacer.step! for sea ice

• Ensured consistent use of DateTime clocks across both components

Testing

• Ran test suite using:
  julia --project -e 'using Pkg; Pkg.test()'

• Verified:

  • Simulations initialize with OC.Clock{DateTime}
  • Time stepping behaves correctly

• Code formatted using JuliaFormatter

---

• I have read and checked the items on the review checklist.

Snapshots of Test passed

[github-actions]

⚠️ Contributor License Agreement required

The following contributor(s) must sign the CLA before this PR can be merged:

• @kavyakapoor420

Please visit https://ecodesign.clima.caltech.edu/cla/ to review and sign the CLA.

How to sign: Authenticate with GitHub then click the "I agree" button.

Once completed, re-run the checks on this PR.

[kavyakapoor420]

@CodeRabbit , give full review

[coderabbitai]

@kavyakapoor420 Sure! Triggering a full review of the PR now. 🐇

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

This PR introduces DateTime-based clock support to sea ice and ocean simulations. It adds a start_date::DateTime field to both ClimaSeaIceSimulation and OceananigansSimulation structs, updates constructors to accept and store this field, and refactors the time-stepping logic to compute absolute times using start_date + Dates.Second(float(t)) instead of relative time arithmetic. The underlying simulation clocks are now initialized with OC.Clock{DateTime}(time = start_date).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• use one ocean remapper #1354: Also modifies OceananigansSimulation and Interfacer.step! implementations, creating potential interaction points with clock and timestep handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding DateTime clock parameter support for CMIP simulations.
Description check	✅ Passed	The description is directly related to the changeset, providing clear purpose, implementation details, and test verification.
Linked Issues check	✅ Passed	All coding requirements from issue #1819 are met: DateTime clock exposed, constructors updated to use DateTime-based clocks, and time-stepping logic refactored accordingly.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the objective of adding DateTime clock support to ocean and sea ice simulations. No unrelated modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ext/ClimaCouplerCMIPExt/clima_seaice.jl`:
- Line 35: The constructor currently stores the optional ECCO init date into
start_date::DateTime and uses it to build OC.Clock{DateTime}, which breaks the
zero-ice fallback and allows a different sea-ice epoch than the coupled/ocean
epoch; fix this by keeping the sea-ice clock epoch tied to the required
coupled/ocean start and treating the ECCO init date as an optional field.
Concretely: revert or change the type of the stored ECCO date to
Union{DateTime,Nothing} (e.g. ecco_init_date::Union{DateTime,Nothing}) instead
of start_date::DateTime, construct OC.Clock{DateTime} from the coupled/ocean
start value (call it coupled_start or use the existing ocean epoch) and store
that clock in the struct, and ensure the constructor accepts start_date=nothing
as the optional ECCO init date but does not feed it into OC.Clock; update all
constructor paths and methods that reference start_date (including uses that
were modified around OC.Clock{DateTime}) to use the separate clock epoch and the
optional ECCO init field accordingly.

In `@ext/ClimaCouplerCMIPExt/oceananigans.jl`:
- Around line 28-29: The constructor currently accepts a generic clock and
stores start_date independently, causing mismatches with step! which rebuilds
wall-clock from sim.start_date; update the constructor (and any places setting
sim.clock or accepting clock, e.g., the types/assignments around start_date and
clock in oceananigans.jl) to enforce the invariant that clock is an
OC.Clock{DateTime} and that clock.time == start_date (throw an ArgumentError if
not), or else remove the clock override entirely so sim.clock is constructed
from start_date; apply the same validation to all other spots mentioned (lines
around the other occurrences) where clock and start_date are set or accepted to
ensure they remain consistent with step!.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: af25fa3e-de07-475a-a31b-4f478ea16cfe

📥 Commits

Reviewing files that changed from the base of the PR and between 3340e9b and d20e110.

📒 Files selected for processing (2)

• ext/ClimaCouplerCMIPExt/clima_seaice.jl
• ext/ClimaCouplerCMIPExt/oceananigans.jl

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Keep the sea-ice clock epoch separate from the optional ECCO init date.

The constructor still accepts start_date = nothing, but the new code now feeds it into OC.Clock{DateTime} and stores it in start_date::DateTime, so the documented zero-ice fallback fails before construction. It also allows a sea-ice start_date different from the coupled ocean epoch, so ocean and ice can advance to different wall-clock times for the same coupler t. The clock start should be fixed to the coupled/ocean start, with the dataset initialization date remaining optional.

Also applies to: 71-82, 159-166, 177-221, 229-233

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ext/ClimaCouplerCMIPExt/clima_seaice.jl` at line 35, The constructor
currently stores the optional ECCO init date into start_date::DateTime and uses
it to build OC.Clock{DateTime}, which breaks the zero-ice fallback and allows a
different sea-ice epoch than the coupled/ocean epoch; fix this by keeping the
sea-ice clock epoch tied to the required coupled/ocean start and treating the
ECCO init date as an optional field. Concretely: revert or change the type of
the stored ECCO date to Union{DateTime,Nothing} (e.g.
ecco_init_date::Union{DateTime,Nothing}) instead of start_date::DateTime,
construct OC.Clock{DateTime} from the coupled/ocean start value (call it
coupled_start or use the existing ocean epoch) and store that clock in the
struct, and ensure the constructor accepts start_date=nothing as the optional
ECCO init date but does not feed it into OC.Clock; update all constructor paths
and methods that reference start_date (including uses that were modified around
OC.Clock{DateTime}) to use the separate clock epoch and the optional ECCO init
field accordingly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Validate clock and start_date as one invariant.

step! now rebuilds wall-clock time from sim.start_date, but the constructor still accepts an arbitrary clock and stores start_date independently. If a caller passes a non-DateTime clock, current_time - sim.ocean.model.clock.time will throw; if they pass a DateTime clock with a different initial time, the first Δt is wrong. Please either require clock isa OC.Clock{DateTime} with clock.time == start_date, or remove the generic clock override here.

Also applies to: 73-74, 163-174, 296-297, 346-349

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ext/ClimaCouplerCMIPExt/oceananigans.jl` around lines 28 - 29, The
constructor currently accepts a generic clock and stores start_date
independently, causing mismatches with step! which rebuilds wall-clock from
sim.start_date; update the constructor (and any places setting sim.clock or
accepting clock, e.g., the types/assignments around start_date and clock in
oceananigans.jl) to enforce the invariant that clock is an OC.Clock{DateTime}
and that clock.time == start_date (throw an ArgumentError if not), or else
remove the clock override entirely so sim.clock is constructed from start_date;
apply the same validation to all other spots mentioned (lines around the other
occurrences) where clock and start_date are set or accepted to ensure they
remain consistent with step!.