← Back to team overview

dolfin team mailing list archive

Re: unsigned int -> std::size_t

 

2012/11/14 Johan Hake <hake.dev@xxxxxxxxx>

> On 11/14/2012 08:52 PM, Anders Logg wrote:
> > On Wed, Nov 14, 2012 at 05:06:13PM +0000, Garth N. Wells wrote:
> >> On Tue, Nov 13, 2012 at 6:40 PM, Anders Logg <logg@xxxxxxxxx> wrote:
> >>> On Tue, Nov 13, 2012 at 06:35:26PM +0000, Garth N. Wells wrote:
> >>>> On Mon, Nov 12, 2012 at 8:39 AM, Anders Logg <logg@xxxxxxxxx> wrote:
> >>>>> On Mon, Nov 12, 2012 at 08:37:47AM +0000, Garth N. Wells wrote:
> >>>>>> On Mon, Nov 12, 2012 at 8:31 AM, Anders Logg <logg@xxxxxxxxx>
> wrote:
> >>>>>>> On Sun, Nov 11, 2012 at 09:33:01PM +0000, Garth N. Wells wrote:
> >>>>>>>> On Sun, Nov 11, 2012 at 7:36 PM, Anders Logg <logg@xxxxxxxxx>
> wrote:
> >>>>>>>>> On Sun, Nov 11, 2012 at 10:22:12AM +0000, Garth N. Wells wrote:
> >>>>>>>>>> On Fri, Oct 26, 2012 at 6:22 AM, Anders Logg <logg@xxxxxxxxx>
> wrote:
> >>>>>>>>>>> On Mon, Oct 22, 2012 at 10:32:11AM +0100, Garth N. Wells wrote:
> >>>>>>>>>>>> We have discussed briefly in the past changing from unsigned
> int
> >>>>>>>>>>>> (typedef uint) to std::size_t. Starting to solve some really
> big
> >>>>>>>>>>>> problems and some changes in Trilinos make it a good time to
> bring
> >>>>>>>>>>>> this up again. Any thoughts or objections to moving to
> std::size_t
> >>>>>>>>>>>> from uint?
> >>>>>>>>>>>
> >>>>>>>>>>> I think this would be a good idea.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> I've started making some unsigned int -> std::size_t changes as
> I
> >>>>>>>>>> restructure mesh partitioning.
> >>>>>>>>>>
> >>>>>>>>>>> I suggest we keep the uint typedef and make it point to size_t.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> I think we should use std::size_t and not uint. std::size_t is
> already
> >>>>>>>>>> a typedef and it conveys an intention: big enough for the
> largest
> >>>>>>>>>> array that can be allocated on a machine.  Also, it's not a
> question
> >>>>>>>>>> of unsigned int or std::size_t - there are places for both.
> >>>>>>>>>
> >>>>>>>>> So we will keep dolfin::uint for stuff like component indices and
> >>>>>>>>> other small integers, and use size_t for everything that can
> >>>>>>>>> potentially be large?
> >>>>>>>>
> >>>>>>>> Yes. I lean towards using 'unsigned int' instead of
> 'dolfin::uint'.
> >>>>>>>
> >>>>>>> Why? To minimize internal typedefs?
> >>>>>>>
> >>>>>>
> >>>>>> Yes. Typing 'unsigned int' in full doesn't bother me.
> >>>>>
> >>>>> I don't feel strongly about it, as long as we're consistent.
> >>>>>
> >>>>>>>>> How about the Mesh? Should we use size_t for stuff like mesh
> >>>>>>>>> connectivity?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> If it can potentially be big, then it should be std::size_t.
> >>>>>>>
> >>>>>>> Is the assumption that global dof numbers need size_t while for
> local
> >>>>>>> entity indices (to a process) it's enough with uint?
> >>>>>>>
> >>>>>>
> >>>>>> I would suggest using std::size_t for local indices.
> >>>>>>
> >>>>>> I've used unsigned int for things like topological and geometric
> >>>>>> dimensions, number of connected entities, number of entities per
> cell,
> >>>>>> etc.
> >>>>>
> >>>>> Is there a performance/memory hit?
> >>>>>
> >>>>
> >>>> There should be no performance hit (some small improvements in places
> >>>> where we will be able to avoid some copying).
> >>>>
> >>>> I've almost finished a transition, with just a few tests to sort out.
> >>>> It's a bit tricky on the Python side because we can't expose uint and
> >>>> std::size_t because the two will clash on 32-bit machines. It also
> >>>> turns out that we have been making assumptions as to the PetscInt type
> >>>> and the Trilinos int type which can't really justify.
> >>>>
> >>>> It will use more memory, but I'll have to test to see how much. I
> >>>> expect that it's just a price that has to be paid to get to really big
> >>>> problems. We can reduce the  std::size_t usage from what I have now if
> >>>> we fix some classes. Some data structures are used to store the cell
> >>>> index, which means that they must be of type std::size_t, and this
> >>>> propagates to parts of the code where std::size_t is not required.
> >>>> std::size_t can be unwound to uint step-by-step.
> >>>
> >>> ok. I agree size_t is a necessary transition to get to bigger
> >>> problems.
> >>>
> >>
> >> I've almost got all the tests running with the size_t transition. I'd
> >> like to merge as soon as the tests pass because the nature of the
> >> branch means that it's impossible to maintain outside of trunk. It
> >> will need some testing to make sure that it works on all platforms. It
> >> may be that bugs in older versions of SWIG (< 2.07) will become
> >> apparent.
> >>
> >> Some Python code will need minor changes (uintc -> unitp) and the C++
> >> code will need to make some dolfin:uint -> std::size_t changes. It
> >> should all be relatively minor.
> >>
> >> There will be a temporary  performance regression in the linear
> >> algebra insertion because I'm copying matrix and vector indices to get
> >> the types right. The plan is to introduce a special typedef to match
> >> the linear algebra backend, i.e. if PETSc is the target backend then
> >> the typdef will be for PetscInt. This will handle the cases where
> >> PETSc is compiled with 32 or 64 bit integers without doing any
> >> copying. It will also eliminate the casts that we presently have in
> >> the linear algebra backends. This typedef will only appear in
> >> GenericTensor::set/add/get and related functions.
> >>
> >> Let me know asap if there are any objections to an imminent merge.
> >
> > No objections, better sooner than later.
>
> Only objection is that we should try to straighten out all CSG related
> buildbot trouble before we make the next big merge.
>
> Maybe that is fixed already?
>

Mostly yes. 7 out of 17 (active) buildbots fail now. One error is clearly
related to the new plotting code (
http://fenicsproject.org:8010/builders/dolfin-trunk-full-squeeze-amd64/builds/216/steps/make%20run_regressiontests/logs/demo.log).
One error (appearing on two bildbots) is possibly related (
http://fenicsproject.org:8010/builders/dolfin-trunk-full-oneiric-amd64/builds/207/steps/make%20run_regressiontests/logs/demo.log).
I'll take a look tomorrow.

Benjamin


>
> Johan
>
> >
> > --
> > Anders
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~dolfin
> > Post to     : dolfin@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~dolfin
> > More help   : https://help.launchpad.net/ListHelp
> >
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~dolfin
> Post to     : dolfin@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~dolfin
> More help   : https://help.launchpad.net/ListHelp
>

References