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

(0.94.1) Refactor SplitExplicitFreeSurface #3894

Merged
merged 161 commits into from
Nov 14, 2024

Conversation

simone-silvestri
Copy link
Collaborator

@simone-silvestri simone-silvestri commented Nov 1, 2024

refactoring of the SplitExplicitFreeSurface to comply with issue #3873

Additional changes included in this PR not described in #3873:

  • the grid halos are extended automatically upon passing a grid with a Connected topology (FullyConnected, RightConnected or LeftConnected) so there is no need to extend the constructor for grids (such as distributed grids) that have connected topologies.
  • All the ExplicitFreeSurface relevant code has been moved in the explicit_free_surface.jl file

@simone-silvestri
Copy link
Collaborator Author

Thanks for the comments, I should have addressed all points.

Comment on lines 61 to 62

Gⁿ⁺¹ = @inbounds C₁ * Gⁿ[i, j, k] - C₂ * G⁻[i, j, k]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Gⁿ⁺¹ = @inbounds C₁ * Gⁿ[i, j, k] - C₂ * G⁻[i, j, k]
not_euler = C₂ != 0
Gⁿ⁺¹ = @inbounds C₁ * Gⁿ[i, j, k] - C₂ * G⁻[i, j, k] * not_euler

multiplying by false eliminates the NaN

(we need this in the other time-stepping code too)

simone-silvestri and others added 3 commits November 14, 2024 17:24
…ces/split_explicit_free_surface.jl

Co-authored-by: Gregory L. Wagner <[email protected]>
…ces/split_explicit_free_surface.jl

Co-authored-by: Gregory L. Wagner <[email protected]>
@simone-silvestri simone-silvestri merged commit bb42ddd into main Nov 14, 2024
46 checks passed
@simone-silvestri simone-silvestri deleted the ss/refactor-split-explicit branch November 14, 2024 19:09
@tomchor
Copy link
Collaborator

tomchor commented Nov 14, 2024

I think was merged with a bump to 0.94.1 before 0.94 was registered. How do we fix this?

@glwagner
Copy link
Member

we have to merge 0.94 retroactively. But yeah, this should have been merged just as 0.94, not 0.94.1 @simone-silvestri .

@navidcy
Copy link
Collaborator

navidcy commented Nov 15, 2024

We don't need to fix. It's OK. We move on.

@simone-silvestri
Copy link
Collaborator Author

Oh sorry, my bad! I messed up a bit here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants