Skip to content

Update OmegaV1GoverningEqns.md#324

Merged
xylar merged 9 commits intoE3SM-Project:developfrom
hyungyukang:hkang/omega/fix-gov-doc
Feb 23, 2026
Merged

Update OmegaV1GoverningEqns.md#324
xylar merged 9 commits intoE3SM-Project:developfrom
hyungyukang:hkang/omega/fix-gov-doc

Conversation

@hyungyukang
Copy link
Copy Markdown

@hyungyukang hyungyukang commented Dec 3, 2025

This PR slightly modifies the Omega-V1 governing equation document OmegaV1GoverningEqns.md.

  • Fix some bugs in formulations
  • Update definition and units in Variable Definitions to match its usage in the document

Compiled version is here.

The code needs to be updated (e.g., changing LayerThickness to PseudoThickness) to match the document. I will open another PR for this.

Checklist

@hyungyukang hyungyukang changed the title Fix bugs in formulations and improve the document Update OmegaV1GoverningEqns.md Dec 3, 2025
@hyungyukang
Copy link
Copy Markdown
Author

The code needs to be updated (e.g., changing LayerThickness to PseudoThickness) to match the document. I will open another PR for this.

I found that this change would require many updates in both Omega and Polaris. I don’t think it’s necessary at this point, so we should keep using LayerThickness for $\tilde{h}$ and GeometricLayerThickness for $h$.

Copy link
Copy Markdown
Collaborator

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

@hyungyukang thanks for your attention to the documentation. This looks good to me.

- Update definition of unit normal since the current definition is
  actually upward normal
@xylar
Copy link
Copy Markdown

xylar commented Dec 15, 2025

@hyungyukang, I also noticed several issues with this document. Most are different from your changes but I think a few may overlap. After we merge your fixes, I would like to rebase #326 and have you take a look at that.

- Remove \rho_0 in (64) and add \tilde to (63)
@xylar
Copy link
Copy Markdown

xylar commented Dec 19, 2025

The code needs to be updated (e.g., changing LayerThickness to PseudoThickness) to match the document. I will open another PR for this.

I found that this change would require many updates in both Omega and Polaris. I don’t think it’s necessary at this point, so we should keep using LayerThickness for $\tilde{h}$ and GeometricLayerThickness for $h$.

I would be strongly in favor or renaming LayerThickness to PseudoThickness to avoid confusion in both Omega and Polaris. I will try to get the ball rolling, even knowing that this will be quite disruptive. It won't be better if we wait.

@xylar
Copy link
Copy Markdown

xylar commented Feb 23, 2026

@hyungyukang and @vanroekel, could we get this finalized an merged? I would like to rebase #326 onto that merge and then I have yet another PR (#351) I'd like to make to revise the way the pressure-gradient term is handled. We ended up needing those revisions for our final implementation.

@hyungyukang
Copy link
Copy Markdown
Author

@vanroekel, could you please review this PR? I’ve approved #326, and I think we can merge both PRs into develop and open another PR later if needed.

Copy link
Copy Markdown
Collaborator

@vanroekel vanroekel left a comment

Choose a reason for hiding this comment

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

Thanks for the ping on this one - approving based on visual inspection

@xylar
Copy link
Copy Markdown

xylar commented Feb 23, 2026

@vanroekel, can you merge this one? (You're the assignee.) Then, I can rebase and merge #326 later today.

@xylar xylar assigned xylar and unassigned vanroekel Feb 23, 2026
@xylar
Copy link
Copy Markdown

xylar commented Feb 23, 2026

@vanroekel, it looks like you didn't have time to get to this, which is totally fine. I'll take care of it now that I'm not on my phone :-).

@xylar xylar merged commit a19ccc0 into E3SM-Project:develop Feb 23, 2026
1 check passed
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.

4 participants