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

Should this be called update_boundary_conditions!? #3945

Open
glwagner opened this issue Nov 21, 2024 · 2 comments
Open

Should this be called update_boundary_conditions!? #3945

glwagner opened this issue Nov 21, 2024 · 2 comments
Labels
cleanup 🧹 Paying off technical debt

Comments

@glwagner
Copy link
Member

We were reading the code and explaining it with @odunbar and ran into this... I think that multiple boundary conditions can be updated, is that right @jagoosw?

@apply_regionally update_boundary_condition!(fields(model), model)

@glwagner glwagner added the cleanup 🧹 Paying off technical debt label Nov 21, 2024
@jagoosw
Copy link
Collaborator

jagoosw commented Nov 22, 2024

Yeah:

function update_boundary_condition!(fields::Tuple, model)
N = length(fields)
ntuple(Val(N)) do n
field = fields[n]
bcs = boundary_conditions(field)
update_boundary_condition!(bcs, field, model)
end
return nothing
end

and
function update_boundary_condition!(bcs::FieldBoundaryConditions, field, model)
update_boundary_condition!(bcs.west, Val(:west), field, model)
update_boundary_condition!(bcs.east, Val(:east), field, model)
update_boundary_condition!(bcs.south, Val(:south), field, model)
update_boundary_condition!(bcs.north, Val(:north), field, model)
update_boundary_condition!(bcs.bottom, Val(:bottom), field, model)
update_boundary_condition!(bcs.top, Val(:top), field, model)
update_boundary_condition!(bcs.immersed, Val(:immersed), field, model)
return nothing
end

do you think there should be two functions update_boundary_conditions! and update_boundary_condition!, or just the former? I guess its consistent with the rest of the API to have two different ones (e.g. fill_halo_regions! uses fill_X_halo! etc.).

@glwagner
Copy link
Member Author

The main question is: what is easier to understand when reading it?

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

No branches or pull requests

2 participants