|[eigen] Two issues in Sparse/RandomSetter.h|
[ Thread Index |
| More lists.tuxfamily.org/eigen Archives
- To: eigen@xxxxxxxxxxxxxxxxxxx
- Subject: [eigen] Two issues in Sparse/RandomSetter.h
- From: Matthew Hancher <mdh@xxxxxxxxxx>
- Date: Wed, 28 Apr 2010 13:36:22 -0700
- Cc: Rich Washington <rwashington@xxxxxxxxxx>
- Dkim-signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=google.com; s=beta; t=1272486985; bh=cyWd6/Xlt3UXUY4RnjYaRuysvwk=; h=MIME-Version:Date:Message-ID:Subject:From:To:Cc:Content-Type; b=E0X6yFaPsTD8VDvIvL2vgwDXaA150Mu3T0gl5rgavkqNix4U+QStwLO34Gdb0NPIC QvrBC/tArULCtlms2j+5Q==
- Domainkey-signature: a=rsa-sha1; s=beta; d=google.com; c=nofws; q=dns; h=mime-version:date:message-id:subject:from:to:cc: content-type:x-system-of-record; b=h9o09SPE4OO1WFUDd3tqiz+/Mangoyj2GBoitOCtbSRqhWvcEBP1uNc6nZiIOPcg4 mCFZEFpybBCCbO98bA44w==
I'm having a couple of isues with Sparse/RandomSetter.h. This is with eigen 2.0.12 and a pretty standard Ubuntu gcc 4.2.4 installation, FWIW, but the problems appear to exist in tip and not be particularly platform-dependent. I will happily open bug(s) about these, but I'd love a quick sanity-check before I do so.
First, despite efforts to support the GNU hash_map extension, it actually fails to do so. In particular, this innocent-looking sequence of includes:
does not compile, complaining:
.../Sparse/RandomSetter.h:162: error: invalid default argument for a template paramter
The problem is that Sparse/RandomSetter.h never defines GnuHashMapTraits, like it does for the Google hash map variants, so when it tries to use it on line 162 bad things happen. It seems to me that the solution is to define GnuHashMapTraits earlier in that file. I can submit a patch to do this if you think I'm on the right track.
Second, that same template argument is documented (at line 146) as defaulting to GoogleSparseHashMapTraits if the Google sparse_hash_map is available, but in fact the implementation ignores that altogether and instead defaults to GoogleDenseHashMapTraits if that is avilable, which doesn't even appear at all in the documented default preference order. It seems to me that the solution there is to pick one preference order (either Dense/Gnu/Std, Sparse/Gnu/Std, or Sparse/Dense/Gnu/Std) and adjust both the documentation comment and the implementation to use that order. I'll happily submit a patch for that too, if someone who understands the implications of changing the default can tell me what the best order is.