Modify

Ticket #7363 (closed Bugs: fixed)

Opened 2 years ago

Last modified 2 years 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 2 years ago.
custom_inplace_merge_fixed.diff Download (33.4 KB) - added by j.ungermann@… 2 years ago.
custom_inplace_merge_r80623.diff Download (32.4 KB) - added by j.ungermann@… 2 years ago.

Change History

comment:1 Changed 2 years ago by guwi17

  • Component changed from None to uBLAS

comment:2 Changed 2 years 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 2 years ago by guwi17

  • Status changed from new to assigned

Changed 2 years ago by anonymous

comment:4 Changed 2 years 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 2 years ago by guwi17

(In [80586])

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

comment:6 Changed 2 years 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 2 years ago by j.ungermann@…

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

Changed 2 years ago by j.ungermann@…

Changed 2 years ago by j.ungermann@…

comment:8 Changed 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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.