More abstract vectors #1252
More abstract vectors #1252
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1252 +/- ##
=======================================
Coverage 94.19% 94.19%
=======================================
Files 40 40
Lines 6662 6662
=======================================
Hits 6275 6275
Misses 387 387 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
KnutAM
left a comment
KnutAM
left a comment
There was a problem hiding this comment.
Just some quick comments due to slack-ping :)
| function celldofs!(global_dofs::AbstractVector{Int}, dh::DofHandler, i::Int) | ||
| @assert isclosed(dh) | ||
| @assert length(global_dofs) == ndofs_per_cell(dh, i) | ||
| unsafe_copyto!(global_dofs, 1, dh.cell_dofs, dh.cell_dofs_offset[i], length(global_dofs)) | ||
| return global_dofs | ||
| end |
There was a problem hiding this comment.
I think allowing AbstractVector is not compatible with using unsafe_copyto! here?
There was a problem hiding this comment.
cc @fredrikekre I guess we need to variants here then? One on Vector with unsafe_copyto and one with just copyto. Is there a reason why this is not simply copyto!?
| node_ids = get_node_ids(cell) | ||
| @inbounds for i in 1:length(x) | ||
| x[i] = get_node_coordinate(grid, node_ids[i]) | ||
| end |
There was a problem hiding this comment.
1:length(x) should be used for AbstractVector (and the use of @inbounds here seems highly questionable, seems like out of bounds can occur by just passing a too long vector x, but that is unrelated).
Both could be fixed though with
| node_ids = get_node_ids(cell) | |
| @inbounds for i in 1:length(x) | |
| x[i] = get_node_coordinate(grid, node_ids[i]) | |
| end | |
| node_ids = get_node_ids(cell) | |
| @boundscheck checkbounds(x, 1:length(node_ids)) | |
| @inbounds for (i, node_id) in enumerate(node_ids) | |
| x[i] = get_node_coordinate(grid, node_id) | |
| end |
| function _cellnodes!(global_nodes::AbstractVector{Int}, cell::AbstractCell) | ||
| @assert length(global_nodes) == nnodes(cell) | ||
| @inbounds for i in 1:length(global_nodes) | ||
| global_nodes[i] = cell.nodes[i] |
There was a problem hiding this comment.
Same as getcoordinates! (needs @boundscheck checkbounds + enumerate) (and while at it, perhaps use get_node_ids?)
This allows e.g. views to be used.