← Back to team overview

dolfin team mailing list archive

Re: unsigned int -> std::size_t

 

On Wed, Nov 14, 2012 at 8:51 PM, Johan Hake <hake.dev@xxxxxxxxx> wrote:
> 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?
>

Buildbots seem to be more-or-less as the were before the CSG push.

All the tests passed on my machine, so I've pushed the changes. It
will be a bit painful for while, but worth it in the end ;).

I made a lot of "uint" -> "sizet" changes for MeshFunctions on the
Python side. We could choose to stick with "uint" and just map it to
std::size_t.

Garth


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


References