When distributing MG DoF indices, we accidentally used 'unsigned int'
instead of 'types::global_dof_index'. That's a bug. We didn't notice
this because we never have more than 4B unknowns on one processor,
and the function was only called in the parallel context to enumerate
the DoFs on the *local* portion of the mesh, before indices were
shifted after communication with other processors. Regardless, it's
worth fixing.
We had a function with a funny comment suggesting that a compiler did not
correctly understand the code. But the comment is wrong: when the only
argument to a function that references a 'dim' or 'spacedim' template
argument is of the form
onst typename DoFHandler<dim,spacedim>::level_cell_iterator &
then 'dim' and 'spacedim' are simply not deducible. That's how C++ works.
The function therefore had a dummy argument of type DoFHandler<dim,spacedim>
that isn't used. That's awkward, but works. But we have a way to deal
with the lack of deducibility that's used elsewhere in the library
namely internal::int2type. Use this approach here as well, and remove
the misleading comment.
Do not derive the ParallelShared policy from the Sequential one.
The derivation was slightly contrived, but is not in fact useful any more since
the code has been refactored in ways that allow breaking the derivation and
just calling in ParallelShared what is also called in Sequential. In fact, the
code may have even become a bit shorter that way.
Remove an argument to a function that was no longer used.
Specifically, it allowed to start enumerating DoFs at something other than zero.
There was a time when we allowed that, but that possibility was removed
several years ago.
The corresponding argument to distribute_mg_dofs() was removed a few days ago,
but it was accidently left on distribute_dofs().
The compiler warns about the line
header_ += gzip::magic::id2; // ID2.
deep in BOOST when writing the header for a gzip-compressed block of data. This
is because (i) gzip::magic::id2 is declared as an int, and (ii) has a value
greater than what a *signed char* can store, i.e., greater than 127. Furthermore,
std::string::operator+ takes a 'char', which may or may not be unsigned but
apparently on my system is signed. So we get a warning about overflow.
The only reasonable way to deal with this is to do the casting explicitly.
David Wells [Sat, 24 Jun 2017 14:35:27 +0000 (10:35 -0400)]
Grammar: clarify obtain vs. attain.
Attain and obtain are nearly, but not quite, synonyms: 'attain' implies
achievement or accomplishment while 'obtain' just implies
ownership. This patch fixes our errant usage of 'attain' (one comment
also had a logic error, which I fixed).
Remove an argument to a function that was no longer used.
Specifically, it allowed to start enumerating DoFs at something other than zero.
There was a time when we allowed that, but that possibility was removed
several years ago.
Let DoFHandler policy classes return the NumberCache also in distribute_mg_dofs().
This follows the same pattern as a previous commit. However, it involves less
drama, with the exception of an innocent assertion that had to be removed
(see the last commit). Other than that, everything seems to work.
The assertion gets in the way of the following commit because it tests
a condition that at the time when we want to call this function is not
yet satisfied. However, I've convinced myself that if the DoFHandler
is truly not set up at all (as the assertion wants to check), that
the calls further down in the function will fail to work as well. In
other words, one will get *some* kind of error.
Let the DoFHandler policy classes' distribute_dofs() return the NumberCache.
In times of move operators, there is really no reason why these functions should
return the NumberCache object by reference, rather than by value. We make this
slightly more efficient by providing (defaulted) move operations to
NumberCache.
However, this change uncovered a subtle problem that sooner or later would
certainly have tripped someone up: If any of the policy classes call
any function that relies on the correctness of the number cache of the
DoFHandler object being worked on, then this is likely going to fail.
Indeed, the implementation of distribute_dofs() for shared triangulations
has this problem by calling some of the DoFTools functions that query
properties of the DoFHandler that aren't all completely consistent
yet internally.
This happened to work -- more or less by design -- because the
NumberCache object that the DoFHandler passes to distribute_dofs()
by reference is the one that belongs to the DoFHandler, and
distribute_dofs() makes sure that it sets the one member that is
necessary to the correct value before calling the DoFTools function.
This can only properly be described as 'brittle': small changes to
the code will make this fail, and it is not obvious why this works
or doesn't.
The only proper solution, obviously, is to not call external functions
from inside distribute_dofs() until all data structures have been set
up completely and are internally consistent. I work around this
by duplicating a small amount of code (specifically, the
DoFTools::get_subdomain_association() and DoFRenumbering::subdomain_wise()
functions), but this effort is made more palatable because these functions
are in fact more general than we need here, and we can throw away a good
amount of code that deals with either sequential or completely
distributed DoFHandlers; we also know that a couple of assertions
in these functions must be satisfied here, so we can eliminate them
as well.
The consequence of this code is slightly more code that is more robust
and contains fewer surprises.
Let the DoFHandler policy classes store references to the DoFHandler.
This is rather than passing this reference to each and every call to the policy functions.
The primary goal of this is so that we can use the Policy base class not only for the
'normal' DoFHandler, but also for the hp::DoFHandler by removing the reference to the
actual class type from the interface of the base class.
Also replace all uses by numbers::invalid_dof_index. I can't see a reason to
have both of these numbers, in particular because the only logical choice is
to have them have the same value.
The functions in question take an IndexSet argument that can either mean
something, or if it is empty, is ignored. In the places where an empty
IndexSet is passed, it actually has size zero, not just zero elements,
but in the existing code, we test whether 'indices.n_elements()==0'.
It turns out that 'n_elements()' is a pretty expensive function to call.
What we really wanted, of course, was 'indices.size()==0', and this
patch makes that change.
I ran all multigrid tests (the functions in question are multigrid
related), and they all succeed.
David Wells [Wed, 21 Jun 2017 02:15:25 +0000 (22:15 -0400)]
Compile TransfiniteInterpolationManifold<1> in the library.
In addition, disable usage of the class by throwing an exception.
This class is not as useful in 1D as it is in higher spatial dimensions, but
compiling it in the library improves generic programming (i.e., one can write a
solver that runs in 1D and 2D and on a 2D execution path can use
TransfiniteInterpolationManifold).
Simplify the logic of CellDataTransferBuffer::(un)pack_data().
Specifically, instead of serializing by hand with memcpy etc, use the BOOST
facilities for serialization. This works for 2 out of 3 members of this class
without problem, and the last one is easy to deal with as well.
Secondly, serialize into a gzip compressed stream to reduce the amount of data to
transfer. This reduces the size of messages significantly; for example, in the
last cycle of step-40, we get the following data -- old packet size on the left,
new (compressed) data size on the right:
6400 1694
8440 2200
8440 2412
6400 1925
7828 2042
6060 1589
1504 583
1504 520
6060 1849
7828 2280
6060 1947
7828 2375
1504 612
76 102
76 104
6400 2025
8440 2537
I did also try to run things with BOOST's bzip2 compressor instead of gzip,
given that bzip2 often does a better job than gzip, but apparently that
isn't the case here as I get the following:
6400 1912
8440 2381
8440 2712
6400 2230
7828 2197
1504 681
6060 2177
6060 1803
1504 556
7828 2678
6060 2285
7828 2738
1504 720
76 122
76 123
6400 2392
8440 2896
The hp::DoFHandler class does not support multilevel DoFs, and so having a
remnant part of the infrastructure for them makes no sense. Removing this
part from the class, however, requires a couple of specializations of
functions in the DoFAccessor class that may have previously compiled,
but would likely have led to segmentation faults if anyone tried to use them.
David Wells [Mon, 19 Jun 2017 20:41:53 +0000 (16:41 -0400)]
Fix two test outputs.
Commit 852db02945e4eedf251d528d701b0c2b10a368c2 removed the redundant
"list" (i.e., previously we printed "List list of ...") from
List::description. This commit updates the output files in the
corresponding tests.
Add a couple of static asserts to SolutionTransfer.
This class (in both of its incarnations) takes both a dimension and a DoFHandler
as template argument. The dimension of the DoFHandler better match the explicitly
given dimension. The added static_assert checks that and prevents instantiation
of the class if that is not the case.
The code suggests that one could update the cell DoF cache, but the cache only
exists for non-MG DoF indices, whereas the function where the comment resides
deals with MG level DoFs.
projects / dealii.git / log