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

Fix few bugs + improve docstring of SplitExplicitFreeSurface #3131

Merged
merged 9 commits into from
Jun 5, 2023

Conversation

navidcy
Copy link
Collaborator

@navidcy navidcy commented Jun 3, 2023

bugs:

  • Δy = minimum_xspacing(grid)
  • some leftover Δt_max was changed to max_Δt
  • when computing substeps via cfl then we need to ensure it's an integer

@navidcy navidcy added bug 🐞 Even a perfect program still has bugs documentation 📜 The sacred scrolls labels Jun 3, 2023
@glwagner
Copy link
Member

glwagner commented Jun 3, 2023

Might be better to contribute this to the broader user API overhaul in #3124 (won't this cause conflicts?)

@navidcy
Copy link
Collaborator Author

navidcy commented Jun 3, 2023

#3124 was merged!

@navidcy
Copy link
Collaborator Author

navidcy commented Jun 3, 2023

@simone-silvestri, what exactly is max_Δt? It's the maximum baroclinic tilmestep, right?

@glwagner
Copy link
Member

glwagner commented Jun 3, 2023

@simone-silvestri, what exactly is max_Δt? It's the maximum baroclinic tilmestep, right?

That's right. Maybe we could have a better name for that.

@navidcy
Copy link
Collaborator Author

navidcy commented Jun 3, 2023

Well I at least added a remark.

@navidcy navidcy changed the title Fix Δy = minimum_ xspacing bug + improve SplitExplicitFreeSurface docstring Fix few bugs + improve docstring of SplitExplicitFreeSurface Jun 4, 2023
@navidcy
Copy link
Collaborator Author

navidcy commented Jun 4, 2023

Perhaps at


we should use Int(floor(...)) to be tad more conservative.

@simone-silvestri
Copy link
Collaborator

Perhaps at

we should use Int(floor(...)) to be tad more conservative.

probably ceil(Int, ...)

@navidcy navidcy merged commit 6c2b7ff into main Jun 5, 2023
@navidcy navidcy deleted the ncc/split-explicit-patch branch June 5, 2023 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Even a perfect program still has bugs documentation 📜 The sacred scrolls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants