Throw informative error if cell vector is type unstable
Fixes #155
Codecov Report
Attention: Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
Project coverage is 96.29%. Comparing base (
9b5c7ed) to head (bd8b593).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/gridtypes/unstructured/unstructured.jl | 50.00% | 6 Missing :warning: |
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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
identity.(vec) can be used to "tighten" the type
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.
So this PR may actually break a lot of code which currently works including Ferrite.
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.