Throw informative error if cell vector is type unstable#156
Throw informative error if cell vector is type unstable#156
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #156 +/- ##
==========================================
- Coverage 96.92% 96.29% -0.64%
==========================================
Files 15 15
Lines 879 891 +12
==========================================
+ Hits 852 858 +6
- Misses 27 33 +6 ☔ View full report in Codecov by Sentry. |
|
|
|
On the other hand, it might be better to show a good error message since this could be a performance trap? E.g. the code in here would essentially copy the array, and the user code is also probably slower than it could be. |
|
Yes, my point of view was that, when user code is already type unstable, then we don't need to care too much about performance and thus making a copy is OK. But it's true that in many cases the user doesn't know their code is type unstable, so showing an error or a warning can make sense. Perhaps we can show an error describing the "correct" way of initialising a vector of cells? |
|
I wonder why this works though? https://github.com/Ferrite-FEM/Ferrite.jl/blob/b56e2c9f6dce430a4fcbffee2246449731df2f93/src/Export/VTK.jl#L118-L122 Is this another code path? |
|
Because of this: # By default, cells are attached to unstructured grids.
grid_type(::Type{<:AbstractMeshCell}) = VTKUnstructuredGrid()This means that, by default, unstructured files (.vtu) are created when the cell type is not fully inferred. This works most of the time, except when a polydata file (.vtp) should be created instead (as in #155). So this PR may actually break a lot of code which currently works including Ferrite. |
|
I hadn't realized that MeshCell isn't concrete anymore. I think the code in Ferrite was written back when it was.
Perhaps we can add some basic downstream CI here similar to e.g. https://github.com/Ferrite-FEM/Ferrite.jl/blob/master/.github/workflows/Downstream.yml . Running the full testsuite of Ferrite is definitely overkill, but we can at least run some basic export stuff perhaps. |
Fixes #155