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

Support the Omega time coordinate #249

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Nov 28, 2024

This merge fixes the manufactured solution analysis task in Omega by using the time coordinate instead of the MPAS-Ocean xtime variable.

It also attempts to use time in the viz step but testing is currently inhibited by #248

Checklist

  • Testing comment in the PR documents testing used to verify the changes

@xylar xylar added ocean Related to ocean tests or analysis Omega PR required The polaris changes won't work with the current Omega submodule and require an update labels Nov 28, 2024
@xylar xylar self-assigned this Nov 28, 2024
@xylar
Copy link
Collaborator Author

xylar commented Nov 28, 2024

Needs #231 and E3SM-Project/Omega#169

@xylar
Copy link
Collaborator Author

xylar commented Nov 28, 2024

I was able to run the manufactured solution test case successfully, including analysis. However, without E3SM-Project/Omega#170, it doesn't behave correctly (as expected) and the convergence rate is terrible.

@xylar
Copy link
Collaborator Author

xylar commented Nov 29, 2024

Viz should be tested after #250 is merged and this gets rebased.

@xylar xylar force-pushed the support-omega-time-coord branch 2 times, most recently from a6e420e to b64edf8 Compare December 4, 2024 18:23
@xylar
Copy link
Collaborator Author

xylar commented Dec 4, 2024

Testing

Tested with manufactured solution on Chrysalis, see #231 (comment)

@xylar xylar added Omega PR finished The polaris changes required an update to the Omega submodule and this is now finished and removed Omega PR required The polaris changes won't work with the current Omega submodule and require an update labels Dec 16, 2024

ssh_model = ds.ssh.isel(Time=-1)
if 'nVertLevels' in ssh_model.dims:
# Omega v0 uses stacked shallow water where ssh as nVertLevels
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Omega v0 uses stacked shallow water where ssh as nVertLevels
# Omega v0 uses stacked shallow water where ssh has nVertLevels

Comment on lines +403 to +407
if model == 'mpas-o':
tidx = time_index_from_xtime(ds_out.xtime.values, time)
else:
# time is seconds since the start of the simulation in Omega
tidx = np.argmin(np.abs(ds_out.Time.values - time))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if model == 'mpas-o':
tidx = time_index_from_xtime(ds_out.xtime.values, time)
else:
# time is seconds since the start of the simulation in Omega
tidx = np.argmin(np.abs(ds_out.Time.values - time))
if model == 'mpas-o':
dt = time_since_start(ds_out.xtime.values)
else:
# time is seconds since the start of the simulation in Omega
dt = ds_out.Time.values
tidx = np.argmin(np.abs(dt - time))

Can we break up time_index_from_xtime into two functions:

def time_index_from_xtime(xtime, dt_target, start_xtime=None):
    dt = time_since_start(ds_out.xtime.values, start_xtime=None)
    time_index = np.argmin(np.abs(np.subtract(dt, dt_target)))
    return time_index

def time_since_start(xtime, start_xtime=None):
    if start_xtime is None:
        start_xtime = xtime[0].decode()

    t0 = datetime.datetime.strptime(start_xtime, '%Y-%m-%d_%H:%M:%S')
    dt = np.zeros((len(xtime),))
    for idx, xt in enumerate(xtime):
        t = datetime.datetime.strptime(xt.decode(),
                                       '%Y-%m-%d_%H:%M:%S')
        dt[idx] = (t - t0).total_seconds()
        return dt

Copy link
Collaborator

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

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

I suggested a minor change, which you can take or leave. I successfully ran the analysis step of manufactured_solution with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ocean Related to ocean tests or analysis Omega PR finished The polaris changes required an update to the Omega submodule and this is now finished
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants