-
Notifications
You must be signed in to change notification settings - Fork 1
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 Float comparison ambiguities from generate_coords
#67
base: main
Are you sure you want to change the base?
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 |
---|---|---|
|
@@ -552,38 +552,31 @@ function generate_coords( | |
model_cell_coords :: Gridap.Arrays.Table{<:VectorValue{Dp,T}} | ||
) where {Ti,Dp,T} | ||
n_corners = maximum(topo_cell_ids.data;init=0) | ||
n_vertices = length(unique(model_cell_coords.data)) | ||
|
||
model_coords = fill(VectorValue(fill(T(Inf),Dp)),n_vertices) | ||
|
||
n_vertices_max = length(model_cell_coords.data) | ||
model_coords = fill(VectorValue(fill(T(Inf),Dp)),n_vertices_max) | ||
for (vertex,coord) in zip(topo_cell_ids.data,model_cell_coords.data) | ||
model_coords[vertex] = min(model_coords[vertex],coord) | ||
end | ||
|
||
# A) Non-periodic case: geometry == topology | ||
if n_corners == n_vertices | ||
return topo_cell_ids, model_coords, model_coords | ||
end | ||
|
||
# B) Periodic case: geometry != topology | ||
topo_coords = model_coords[1:n_corners] | ||
|
||
current = n_corners | ||
model_cell_ids = copy(topo_cell_ids) | ||
new_nodes = Dict{VectorValue{Dp,T},Ti}() | ||
n_vertices = n_corners | ||
model_cell_ids = Gridap.Arrays.Table(copy(topo_cell_ids.data),copy(topo_cell_ids.ptrs)) | ||
for (k,(vertex,coord)) in enumerate(zip(topo_cell_ids.data,model_cell_coords.data)) | ||
if coord != topo_coords[vertex] | ||
if haskey(new_nodes,coord) | ||
model_cell_ids.data[k] = new_nodes[coord] | ||
if norm(coord-model_coords[vertex]) > eps(T) | ||
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. Should we use here the relative error instead of the absolute one? |
||
pos = findfirst(x -> norm(x-coord) < eps(T), model_coords[n_corners+1:n_vertices]) | ||
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.
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. |
||
if !isnothing(pos) | ||
model_cell_ids.data[k] = n_corners+pos | ||
else | ||
current += 1 | ||
model_coords[current] = coord | ||
new_nodes[coord] = current | ||
model_cell_ids.data[k] = current | ||
n_vertices += 1 | ||
model_coords[n_vertices] = coord | ||
model_cell_ids.data[k] = n_vertices | ||
end | ||
end | ||
end | ||
@assert current == n_vertices | ||
@debug "function generate_coords :: n_vertices=$n_vertices, n_corners=$n_corners." | ||
|
||
resize!(model_coords,n_vertices) | ||
topo_coords = (n_vertices == n_corners) ? model_coords : model_coords[1:n_corners] | ||
return model_cell_ids, model_coords, topo_coords | ||
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 have a more general question here regarding the need for HOWEVER, if we are separating among topology and geometry, etc. Why do we need the coordinates of the corners in 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. Also ... how is the coordinate of a corner defined mathematically and why? Right now, the code is assigning (arbitrarily?) the minimum coordinate among the coordinates of the all vertices that clash into the same corner. 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. No idea, it might never be used. But it is required, so we might as well give it something consistent, right? |
||
end | ||
|
||
|
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.
I would improve the naming convention.
E.g.
topo_cell_ids => cell_corners
model_cell_coords => cell_vertices_coords
model_coords => vertex_coords
model_cell_ids => cell_vertices
topo_coords => corner_coords