Do not a priori allocate memory for objects that will later be re-sized. #12860
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bangerth
commented
Oct 21, 2021
source/numerics/data_out_faces.cc
Outdated
| // quadrature points as well | ||
| patch.data.reinit(data.n_datasets + dim, patch.data.size(1)); | ||
| patch.data.reinit(data.n_datasets + dim, q_points.size()); |
There was a problem hiding this comment.
This used to work because we had previously sized things correctly for the faces of hypercubes. But it's wrong for other cell types and instead of referencing the previous second dimension of the object, we just need to use the correct number of quadrature points.
source/numerics/data_out_faces.cc
Outdated
| @@ -395,9 +394,6 @@ DataOutFaces<dim, spacedim>::build_patches( | |||
| update_flags); | |||
| DataOutBase::Patch<patch_dim, patch_spacedim> sample_patch; | |||
| sample_patch.n_subdivisions = n_subdivisions; | |||
| sample_patch.data.reinit(n_datasets, | |||
| Utilities::fixed_power<patch_dim>(n_subdivisions + | |||
| 1)); | |||
There was a problem hiding this comment.
This is the change described in the introduction of the PR.
peterrum
approved these changes
Oct 22, 2021
peterrum
reviewed
Oct 22, 2021
source/numerics/data_out_faces.cc
Outdated
| @@ -105,8 +105,7 @@ DataOutFaces<dim, spacedim>::build_one_patch( | |||
| // on cells, not faces, so transform the face vertex to a cell vertex, that | |||
| // to a unit cell vertex and then, finally, that to the mapped vertex. In | |||
| // most cases this complicated procedure will be the identity. | |||
| for (unsigned int vertex = 0; | |||
| vertex < GeometryInfo<dim - 1>::vertices_per_cell; | |||
| for (unsigned int vertex = 0; vertex < cell->face(face_number)->n_vertices(); | |||
There was a problem hiding this comment.
There is not cell and face_number here.
There was a problem hiding this comment.
It is cell_and_face->first and ->second, I guess.
kronbichler
approved these changes
Oct 22, 2021
|
Yes, I had taken that from the same branch as #12859. I guess this has to wait for that patch... |
|
This was made obsolete by one of the other patches :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here, this is the case for a CopyData object to be used in WorkStream: We have
to give WorkStrean::run() an exemplar of these CopyData objects from which
it will create a concrete object for each task (=cell or face) that will
then be worked on. We initialize the exemplar's '.data' object to a concrete
size, but this size will later be ignored and instead we resize things
to whatever we actually need in one place because we don't really know
what the correct size is going to be a priori. As a consequence, just omit
the original sizing and leave the 'exemplar.data' object empty until the
point where we know the size for a concrete object that we obtain by
copying the examplar.
As a side note: This is in DataOutFaces. The DataOut class doesn't do the
initial sizing -- I suspect that we had that code at some point in the past
and got rid of the initial sizing a few years ago but didn't make the same
change for DataOutFaces. It is also possible that that change was made
as part of the simplex transition in DataOut, but DataOutFaces was never
actually transitioned and doesn't work for simplices right now. (That's
what I'm working on right now, of course.)
/rebuild