Modify

Ticket #7363 (closed Bugs: fixed)

Opened 20 months ago

Last modified 19 months ago

coordinate_matrix::sort() fails with gcc 4.7

Reported by: guwi17 Owned by: guwi17
Milestone: Boost 1.52.0 Component: uBLAS
Version: Boost 1.51.0 Severity: Problem
Keywords: ublas coordinate_matrix Cc: j.ungermann@…

Description

coordinate_matrix::sort() fails with gcc 4.7 because std::inplace_merge requires the provided iterator's method operator*() to really return a reference. The current implementation of index_triple's iterator however returns a proxy (which does not comply to the random_access iterator concept).

(The same problem applies to zip_iterator, see e.g. http://www.boost.org/doc/libs/1_48_0/libs/iterator/doc/new-iter-concepts.html#changes-to-algorithm-requirements )

Attachments

custom_inplace_merge.diff Download (33.5 KB) - added by anonymous 19 months ago.
custom_inplace_merge_fixed.diff Download (33.4 KB) - added by j.ungermann@… 19 months ago.
custom_inplace_merge_r80623.diff Download (32.4 KB) - added by j.ungermann@… 19 months ago.

Change History

comment:1 Changed 20 months ago by guwi17

  • Component changed from None to uBLAS

comment:2 Changed 20 months ago by guwi17

(In [80487]) * boost/numeric/ublas/matrix_sparse.hpp: see #7363, add new define BOOST_UBLAS_COO_ALWAYS_DO_FULL_SORT in order to force a full sort instead of partial sort + inplace merge

  • libs/numeric/ublas/test/Jamfile.v2: add test to see effect of new define

comment:3 Changed 19 months ago by guwi17

  • Status changed from new to assigned

Changed 19 months ago by anonymous

comment:4 Changed 19 months ago by j.ungermann@…

I just attached a patch (as anonymous, sorry) hopefully fixing the problem.

Two custom inplace_merge routines were added to coordinate_vector and coordinate_matrix that replace the problematic STL version.

This could be generalized, but this requires an astonishing amount of templated code to properly pull of, so this copy-paste version seems preferable.

In addition two new test files were added that exercise the coordinate types and check on sortedness of indice and correctness.

I compiled and executed the tests, so some additional changes are likely required to automatically execute them. (I am unfamiliar with the boost test system and familiarizing me with that would have create delayed the patch).

comment:5 Changed 19 months ago by guwi17

(In [80586])

  • libs/numeric/ublas/test/Jamfile.v2: see #7363 - separate test of COO matrix

comment:6 Changed 19 months ago by guwi17

(In [80591])

  • boost/numeric/ublas/vector_sparse.hpp: add ifdef BOOST_UBLAS_COO_ALWAYS_DO_FULL_SORT to coordinate_vector::sort() (similar to coordinate_matrix::sort()), see #7363
  • libs/numeric/ublas/test/Jamfile.v2: use COO_ALWAYS_DO_FULL_SORT in test_assign because we do not want to test performance here

comment:7 Changed 19 months ago by j.ungermann@…

Ticket #4862 is a duplicate of this one (obvious from the attached error output in that ticket).

Changed 19 months ago by j.ungermann@…

Changed 19 months ago by j.ungermann@…

comment:8 Changed 19 months ago by j.ungermann@…

I provided two new patch files. The first one with "_fixed" fixes a danging include statement that should generate a compilation error. The second one is the same patch but against the most recent revision 80623 that I just checked out. The original patch was partly against an older version.

comment:9 Changed 19 months ago by guwi17

(In [80625])

  • boost/numeric/ublas/vector_sparse.hpp, boost/numeric/ublas/matrix_sparse.hpp: see #7363, applied patch (added inplace_merge)
  • libs/numeric/ublas/test/Jamfile.v2, libs/numeric/ublas/test/test_coordinate_vector_inplace_merge.cpp, libs/numeric/ublas/test/test_coordinate_matrix_inplace_merge.cpp: add test for COO::sort()

comment:10 Changed 19 months ago by guwi17

(In [80643])

  • boost/numeric/ublas/storage.hpp: see #7363, remove boost::noncopyable from index_pair and index_triple because both classes can be copied

comment:11 Changed 19 months ago by guwi17

(In [80645]) merged [80483],[80487],[80586],[80591],[80592],[80599],[80600],[80624],[80625],[80643],[80644] into release:

  • fix #7296 (hopefully ;-)
  • see #7363 (must wait for a full test cycle before closing this task)

comment:12 Changed 19 months ago by guwi17

(In [80649])

  • libs/numeric/ublas/doc/release_notes.htm, libs/numeric/ublas/doc/index.htm: see #7363, update release notes and docs

comment:13 Changed 19 months ago by guwi17

  • Status changed from assigned to closed
  • Resolution set to fixed

tests are green on release branch

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.