-
Notifications
You must be signed in to change notification settings - Fork 114
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
Remove ndims
where not necessary
#2081
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ function rebalance_solver!(u_ode::AbstractVector, mesh::TreeMesh{2}, equations, | |
end | ||
|
||
resize!(u_ode, | ||
nvariables(equations) * nnodes(dg)^ndims(mesh) * nelements(dg, cache)) | ||
nvariables(equations) * nnodes(dg)^2 * nelements(dg, cache)) | ||
u = wrap_array_native(u_ode, mesh, equations, dg, cache) | ||
|
||
# Get new list of leaf cells | ||
|
@@ -119,7 +119,7 @@ function refine!(u_ode::AbstractVector, adaptor, mesh::Union{TreeMesh{2}, P4estM | |
reinitialize_containers!(mesh, equations, dg, cache) | ||
|
||
resize!(u_ode, | ||
nvariables(equations) * nnodes(dg)^ndims(mesh) * nelements(dg, cache)) | ||
nvariables(equations) * nnodes(dg)^2 * nelements(dg, cache)) | ||
u = wrap_array(u_ode, mesh, equations, dg, cache) | ||
|
||
# Loop over all elements in old container and either copy them or refine them | ||
|
@@ -144,7 +144,7 @@ function refine!(u_ode::AbstractVector, adaptor, mesh::Union{TreeMesh{2}, P4estM | |
|
||
# Increment `element_id` on the refined mesh with the number | ||
# of children, i.e., 4 in 2D | ||
element_id += 2^ndims(mesh) | ||
element_id += 4 | ||
else | ||
if mesh isa P4estMesh | ||
# Copy old element data to new element container and remove Jacobian scaling | ||
|
@@ -165,13 +165,13 @@ function refine!(u_ode::AbstractVector, adaptor, mesh::Union{TreeMesh{2}, P4estM | |
# the counter `element_id` can have two different values at the end. | ||
@assert element_id == | ||
nelements(dg, cache) + | ||
1||element_id == nelements(dg, cache) + 2^ndims(mesh) "element_id = $element_id, nelements(dg, cache) = $(nelements(dg, cache))" | ||
1||element_id == nelements(dg, cache) + 2^2 "element_id = $element_id, nelements(dg, cache) = $(nelements(dg, cache))" | ||
end # GC.@preserve old_u_ode old_inverse_jacobian | ||
|
||
# Sanity check | ||
if mesh isa TreeMesh && isperiodic(mesh.tree) && nmortars(cache.mortars) == 0 && | ||
!mpi_isparallel() | ||
@assert ninterfaces(cache.interfaces)==ndims(mesh) * nelements(dg, cache) ("For $(ndims(mesh))D and periodic domains and conforming elements, the number of interfaces must be $(ndims(mesh)) times the number of elements") | ||
@assert ninterfaces(cache.interfaces)==2 * nelements(dg, cache) ("For 2D and periodic domains and conforming elements, the number of interfaces must be 2 times the number of elements") | ||
end | ||
|
||
return nothing | ||
|
@@ -193,8 +193,8 @@ function refine!(u_ode::AbstractVector, adaptor, | |
# Sanity check | ||
if mesh isa TreeMesh && isperiodic(mesh.tree) && nmortars(cache.mortars) == 0 && | ||
!mpi_isparallel() | ||
@assert ninterfaces(cache_parabolic.interfaces)==ndims(mesh) * | ||
nelements(dg, cache_parabolic) ("For $(ndims(mesh))D and periodic domains and conforming elements, the number of interfaces must be $(ndims(mesh)) times the number of elements") | ||
@assert ninterfaces(cache_parabolic.interfaces)==2 * | ||
nelements(dg, cache_parabolic) ("For 2D and periodic domains and conforming elements, the number of interfaces must be 2 times the number of elements") | ||
end | ||
|
||
return nothing | ||
|
@@ -313,7 +313,7 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
reinitialize_containers!(mesh, equations, dg, cache) | ||
|
||
resize!(u_ode, | ||
nvariables(equations) * nnodes(dg)^ndims(mesh) * nelements(dg, cache)) | ||
nvariables(equations) * nnodes(dg)^2 * nelements(dg, cache)) | ||
u = wrap_array(u_ode, mesh, equations, dg, cache) | ||
Comment on lines
315
to
317
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think parts like these become much harder to understand with this PR. What do you think, @sloede? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I have to agree with Hendrik. The advantage of explicitly adding I see the argument for more "elegant" code (avoiding the impression of having a varying dimension). However, I've seen too many codes where "magic" constants appear out of nowhere, and everyone reading it after 3 years has to re-understand what the intent/thought process of the original author was. IMHO, the current version is concise and self-explanatory in terms of what is being done, thus I'd prefer to keep it as it is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To throw in my two cents. I want to keep the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I will close this PR then. Opinions about the status of the issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would close it in light of the comments above |
||
|
||
# Loop over all elements in old container and either copy them or coarsen them | ||
|
@@ -330,7 +330,7 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
# If an element is to be removed, sanity check if the following elements | ||
# are also marked - otherwise there would be an error in the way the | ||
# cells/elements are sorted | ||
@assert all(to_be_removed[old_element_id:(old_element_id + 2^ndims(mesh) - 1)]) "bad cell/element order" | ||
@assert all(to_be_removed[old_element_id:(old_element_id + 2^2 - 1)]) "bad cell/element order" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One could move |
||
|
||
# Coarsen elements and store solution directly in new data structure | ||
coarsen_elements!(u, element_id, old_u, old_element_id, | ||
|
@@ -349,7 +349,7 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
# Increment `element_id` on the coarsened mesh by one and `skip` = 3 in 2D | ||
# because 4 children elements become 1 parent element | ||
element_id += 1 | ||
skip = 2^ndims(mesh) - 1 | ||
skip = 3 | ||
else | ||
if mesh isa P4estMesh | ||
# Copy old element data to new element container and remove Jacobian scaling | ||
|
@@ -372,7 +372,7 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
# Sanity check | ||
if mesh isa TreeMesh && isperiodic(mesh.tree) && nmortars(cache.mortars) == 0 && | ||
!mpi_isparallel() | ||
@assert ninterfaces(cache.interfaces)==ndims(mesh) * nelements(dg, cache) ("For $(ndims(mesh))D and periodic domains and conforming elements, the number of interfaces must be $(ndims(mesh)) times the number of elements") | ||
@assert ninterfaces(cache.interfaces)==2 * nelements(dg, cache) ("For 2D and periodic domains and conforming elements, the number of interfaces must be 2 times the number of elements") | ||
end | ||
|
||
return nothing | ||
|
@@ -394,8 +394,8 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
# Sanity check | ||
if mesh isa TreeMesh && isperiodic(mesh.tree) && nmortars(cache.mortars) == 0 && | ||
!mpi_isparallel() | ||
@assert ninterfaces(cache_parabolic.interfaces)==ndims(mesh) * | ||
nelements(dg, cache_parabolic) ("For $(ndims(mesh))D and periodic domains and conforming elements, the number of interfaces must be $(ndims(mesh)) times the number of elements") | ||
@assert ninterfaces(cache_parabolic.interfaces)==2 * | ||
nelements(dg, cache_parabolic) ("For 2D and periodic domains and conforming elements, the number of interfaces must be 2 times the number of elements") | ||
end | ||
|
||
return nothing | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,7 @@ function refine!(u_ode::AbstractVector, adaptor, mesh::Union{TreeMesh{3}, P4estM | |
reinitialize_containers!(mesh, equations, dg, cache) | ||
|
||
resize!(u_ode, | ||
nvariables(equations) * nnodes(dg)^ndims(mesh) * nelements(dg, cache)) | ||
nvariables(equations) * nnodes(dg)^3 * nelements(dg, cache)) | ||
u = wrap_array(u_ode, mesh, equations, dg, cache) | ||
|
||
# Loop over all elements in old container and either copy them or refine them | ||
|
@@ -77,7 +77,7 @@ function refine!(u_ode::AbstractVector, adaptor, mesh::Union{TreeMesh{3}, P4estM | |
|
||
# Increment `element_id` on the refined mesh with the number | ||
# of children, i.e., 8 in 3D | ||
element_id += 2^ndims(mesh) | ||
element_id += 8 | ||
else | ||
if mesh isa P4estMesh | ||
# Copy old element data to new element container and remove Jacobian scaling | ||
|
@@ -98,12 +98,12 @@ function refine!(u_ode::AbstractVector, adaptor, mesh::Union{TreeMesh{3}, P4estM | |
# the counter `element_id` can have two different values at the end. | ||
@assert element_id == | ||
nelements(dg, cache) + | ||
1||element_id == nelements(dg, cache) + 2^ndims(mesh) "element_id = $element_id, nelements(dg, cache) = $(nelements(dg, cache))" | ||
1||element_id == nelements(dg, cache) + 8 "element_id = $element_id, nelements(dg, cache) = $(nelements(dg, cache))" | ||
end # GC.@preserve old_u_ode old_inverse_jacobian | ||
|
||
# Sanity check | ||
if mesh isa TreeMesh && isperiodic(mesh.tree) && nmortars(cache.mortars) == 0 | ||
@assert ninterfaces(cache.interfaces)==ndims(mesh) * nelements(dg, cache) ("For $(ndims(mesh))D and periodic domains and conforming elements, the number of interfaces must be $(ndims(mesh)) times the number of elements") | ||
@assert ninterfaces(cache.interfaces)==3 * nelements(dg, cache) ("For 3D and periodic domains and conforming elements, the number of interfaces must be 3 times the number of elements") | ||
end | ||
|
||
return nothing | ||
|
@@ -229,7 +229,7 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
reinitialize_containers!(mesh, equations, dg, cache) | ||
|
||
resize!(u_ode, | ||
nvariables(equations) * nnodes(dg)^ndims(mesh) * nelements(dg, cache)) | ||
nvariables(equations) * nnodes(dg)^3 * nelements(dg, cache)) | ||
u = wrap_array(u_ode, mesh, equations, dg, cache) | ||
|
||
# Loop over all elements in old container and either copy them or coarsen them | ||
|
@@ -250,7 +250,7 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
# If an element is to be removed, sanity check if the following elements | ||
# are also marked - otherwise there would be an error in the way the | ||
# cells/elements are sorted | ||
@assert all(to_be_removed[old_element_id:(old_element_id + 2^ndims(mesh) - 1)]) "bad cell/element order" | ||
@assert all(to_be_removed[old_element_id:(old_element_id + 2^3 - 1)]) "bad cell/element order" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One could move |
||
|
||
# Coarsen elements and store solution directly in new data structure | ||
coarsen_elements!(u, element_id, old_u, old_element_id, adaptor, | ||
|
@@ -269,7 +269,7 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
# Increment `element_id` on the coarsened mesh by one and `skip` = 7 in 3D | ||
# because 8 children elements become 1 parent element | ||
element_id += 1 | ||
skip = 2^ndims(mesh) - 1 | ||
skip = 7 | ||
else | ||
if mesh isa P4estMesh | ||
# Copy old element data to new element container and remove Jacobian scaling | ||
|
@@ -291,7 +291,7 @@ function coarsen!(u_ode::AbstractVector, adaptor, | |
|
||
# Sanity check | ||
if mesh isa TreeMesh && isperiodic(mesh.tree) && nmortars(cache.mortars) == 0 | ||
@assert ninterfaces(cache.interfaces)==ndims(mesh) * nelements(dg, cache) ("For $(ndims(mesh))D and periodic domains and conforming elements, the number of interfaces must be $(ndims(mesh)) times the number of elements") | ||
@assert ninterfaces(cache.interfaces)==3 * nelements(dg, cache) ("For 3D and periodic domains and conforming elements, the number of interfaces must be 3 times the number of elements") | ||
end | ||
|
||
return nothing | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One could move
skip = 1
already up here