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

add hyperbolic tangent stretching #1930

Merged
merged 1 commit into from
Aug 15, 2024
Merged

add hyperbolic tangent stretching #1930

merged 1 commit into from
Aug 15, 2024

Conversation

szy21
Copy link
Member

@szy21 szy21 commented Aug 13, 2024

Closes #1927. Also fixes a bug in stretching for non-zero z_bottom.

  • Code follows the style guidelines OR N/A.
  • Unit tests are included OR N/A.
  • Code is exercised in an integration test OR N/A. (This will be tested in ClimaAtmos)
  • Documentation has been added/updated OR N/A.

@szy21 szy21 force-pushed the zs/stretching branch 5 times, most recently from 316f6f7 to a0d4694 Compare August 13, 2024 21:46
@szy21 szy21 marked this pull request as ready for review August 13, 2024 21:46
Copy link
Contributor

@tapios tapios left a comment

Choose a reason for hiding this comment

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

We should keep the terrain-following and stretching transformations cleanly separated.

src/Meshes/interval.jl Show resolved Hide resolved
src/Meshes/interval.jl Show resolved Hide resolved
src/Meshes/interval.jl Show resolved Hide resolved
src/Meshes/interval.jl Outdated Show resolved Hide resolved
src/Meshes/interval.jl Outdated Show resolved Hide resolved
@szy21 szy21 force-pushed the zs/stretching branch 2 times, most recently from 68aebf8 to 6d8237c Compare August 13, 2024 23:34
Copy link
Member

@charleskawczynski charleskawczynski left a comment

Choose a reason for hiding this comment

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

Code changes look good overall, I agree. I left some comments, it would be good to generalize the coordinate warping, since that will change the interface.

@szy21 szy21 force-pushed the zs/stretching branch 2 times, most recently from 79a0a50 to c542ba5 Compare August 14, 2024 18:29
@szy21
Copy link
Member Author

szy21 commented Aug 14, 2024

To clarify, we are not hard coding linear terrain-following coordinates here. IntervalMesh deals with stretching, and the linear function is for the cases with non-zero z_bottom. Warping is done in Hyposgraphy, which has generalized terrain-following coordinates.

The code can be cleaned up though, I agree. We will refactor it in separate PRs.

@szy21 szy21 added enhancement New feature or request bugfix labels Aug 14, 2024
@szy21 szy21 force-pushed the zs/stretching branch 3 times, most recently from 6608872 to 0abee99 Compare August 15, 2024 00:53
@szy21 szy21 merged commit 230d4a2 into main Aug 15, 2024
18 of 21 checks passed
@szy21 szy21 deleted the zs/stretching branch August 15, 2024 04:43
@@ -305,7 +310,7 @@ function IntervalMesh(
h =
h_bottom .+
(ζ_n .- ζ_n[1]) * (h_top - h_bottom) / (ζ_n[end - 1] - ζ_n[1])
faces = (z_bottom + (z_top - z_bottom)) * exp_stretch.(ζ_n, h)
faces = z_bottom .+ (z_top - z_bottom) * exp_stretch.(ζ_n, h)
Copy link
Member Author

Choose a reason for hiding this comment

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

@charleskawczynski Here is a bug fix. Would this be a breaking change? This technically changes the behavior of the vertical mesh, but since we always use z_bottom = 0 in ClimaAtmos, it wouldn't change the results. Just want to make sure if the next climacore release should be a minor or patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add hyperbolic tangent stretching
3 participants