Re: [Yade-dev] Yade is not compatible with CGAL_4.13
Hello Janek, thanks for the fix! I have checked it and it really resolves the problem with the newer CGAL. I will reactivate CGAL-function for the Debian-build again. Regards Anton Am Mo., 7. Jan. 2019 um 13:44 Uhr schrieb Janek Kozicki : > > Bruno Chareyre said: (by the date of Sun, 6 Jan 2019 15:34:11 +0100) > > > > Hi Janek, > > > I think that was the right fix, thanks. > > > Note that yade is not using the periodic triangulation implemented in CGAL > > > [1] so there is no question on that point afaik. > > > Bruno > > good, great to know. During the migration I am not committing > anything to gitlab. I also will wait until you synchronize my commits > on github to gitlab :) > > > > [1] that's because yade included periodicity years before cgal. I actually > > > implemented a periodicity based on cgal's non-periodic triangulation. Cgal > > > devs made periodic regular triangulation available more recently - > > > triggered by me for a part - but it's still not used in yade. Hopefully it > > > will be used one day but the refactoring it implies is a bit daunting. > > Before we even think about doing this, let's first make sure that > whatever we modify has working tests :) > > > I forgot to answer the test coverage question: no, move() is not covered by > > any test, even indirectly. > > The function is not used in other parts of the code currently. > > That is one of the test cases which I want to discuss in the > compatibility thread :) > > -- > Janek Kozicki > > ___ > Mailing list: https://launchpad.net/~yade-dev > Post to : yade-dev@lists.launchpad.net > Unsubscribe : https://launchpad.net/~yade-dev > More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~yade-dev Post to : yade-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~yade-dev More help : https://help.launchpad.net/ListHelp
Re: [Yade-dev] Yade is not compatible with CGAL_4.13
Bruno Chareyre said: (by the date of Sun, 6 Jan 2019 15:34:11 +0100) > > Hi Janek, > > I think that was the right fix, thanks. > > Note that yade is not using the periodic triangulation implemented in CGAL > > [1] so there is no question on that point afaik. > > Bruno good, great to know. During the migration I am not committing anything to gitlab. I also will wait until you synchronize my commits on github to gitlab :) > > [1] that's because yade included periodicity years before cgal. I actually > > implemented a periodicity based on cgal's non-periodic triangulation. Cgal > > devs made periodic regular triangulation available more recently - > > triggered by me for a part - but it's still not used in yade. Hopefully it > > will be used one day but the refactoring it implies is a bit daunting. Before we even think about doing this, let's first make sure that whatever we modify has working tests :) > I forgot to answer the test coverage question: no, move() is not covered by > any test, even indirectly. > The function is not used in other parts of the code currently. That is one of the test cases which I want to discuss in the compatibility thread :) -- Janek Kozicki ___ Mailing list: https://launchpad.net/~yade-dev Post to : yade-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~yade-dev More help : https://help.launchpad.net/ListHelp
Re: [Yade-dev] Yade is not compatible with CGAL_4.13
I forgot to answer the test coverage question: no, move() is not covered by
any test, even indirectly.
The function is not used in other parts of the code currently.
B
On Sun, 6 Jan 2019 at 13:44, Bruno Chareyre
wrote:
> Hi Janek,
> I think that was the right fix, thanks.
> Note that yade is not using the periodic triangulation implemented in CGAL
> [1] so there is no question on that point afaik.
> Bruno
>
> [1] that's because yade included periodicity years before cgal. I actually
> implemented a periodicity based on cgal's non-periodic triangulation. Cgal
> devs made periodic regular triangulation available more recently -
> triggered by me for a part - but it's still not used in yade. Hopefully it
> will be used one day but the refactoring it implies is a bit daunting.
>
>
>
> On Fri, 4 Jan 2019 at 19:46, Janek Kozicki wrote:
>
>> It appears that this function move_point(…) had long been deprecated.
>> I guess that's got something to do with previous modifications where we
>> had to use weighted points differently.
>>
>> I propose to modify this line into calling move(…) instead.
>>
>> This isn't change like the previous one where the C++ template
>> structure was changed and I had to dwell deeply into code to recover
>> the original implementation. This is a "small" change of the code
>> which I did not write, so I suppose that Bruno I will need you to
>> certify this.
>>
>> Let's see the code in old version:
>> /usr/include/CGAL/Regular_triangulation_3.h
>>
>> #ifndef CGAL_NO_DEPRECATED_CODE
>> template < class Gt, class Tds, class Lds >
>> typename Delaunay_triangulation_3::Vertex_handle
>> Delaunay_triangulation_3::
>> move_point(Vertex_handle v, const Point & p)
>> {
>> CGAL_triangulation_precondition(! is_infinite(v));
>> CGAL_triangulation_expensive_precondition(is_vertex(v));
>>
>> // Dummy implementation for a start.
>>
>> // Remember an incident vertex to restart
>> // the point location after the removal.
>> Cell_handle c = v->cell();
>> Vertex_handle old_neighbor = c->vertex(c->index(v) == 0 ? 1 : 0);
>> CGAL_triangulation_assertion(old_neighbor != v);
>>
>> remove(v);
>>
>> if (dimension() <= 0)
>> return insert(p);
>> return insert(p, old_neighbor->cell());
>> }
>> #endif
>>
>>
>> And the code in new version:
>> /usr/include/CGAL/Regular_triangulation_3.h
>>
>> template
>> typename Regular_triangulation_3::Vertex_handle
>> Regular_triangulation_3::
>> move(Vertex_handle v, const Weighted_point& p)
>> {
>> CGAL_triangulation_precondition(!is_infinite(v));
>> if(v->point() == p)
>> return v;
>>
>> Self tmp;
>> Vertex_remover remover(tmp);
>> Vertex_inserter inserter(*this);
>> return Tr_Base::move(v,p,remover,inserter);
>> }
>>
>> Which calls tr_Base::move(…) from file:
>> /usr/include/CGAL/Triangulation_3.h
>>
>> template < class Gt, class Tds, class Lds >
>> template < class VertexRemover, class VertexInserter >
>> typename Triangulation_3::Vertex_handle
>> Triangulation_3::
>> move(Vertex_handle v, const Point& p,
>> VertexRemover& remover, VertexInserter& inserter)
>> {
>> CGAL_assertion(remover.hidden_points_begin() ==
>> remover.hidden_points_end());
>> CGAL_triangulation_precondition(!is_infinite(v));
>>
>> if(v->point() == p)
>> return v;
>>
>> Vertex_handle w = move_if_no_collision(v,p,remover,inserter);
>> if(w != v)
>> {
>> remove(v, remover);
>> return w;
>> }
>>
>> return v;
>> }
>>
>> Also let's look at the documentation for move_point:
>>
>>
>> https://doc.cgal.org/latest/Periodic_3_triangulation_3/classCGAL_1_1Periodic__3__Delaunay__triangulation__3.html#aa06932079eb7cf6eee8c5c9998088e34
>>
>> It says that it is just a little more optimized regular move. So it
>> may be not surprise that those codes are not exactly the same. Maybe
>> move_point became obsolete, because they removed the inefficiency
>> that was present in older versions.
>>
>> Note that this function was NOT removed from periodic triangulations.
>> This may be important since yade works in periodic too. So perhaps in
>> Tesselation.ipp there should be a separate move(…) function for
>> periodic cases, to use the old move_point function again when it is
>> possible. Bruno?
>>
>> The compilation passed all tests and checks. But do those tests cover
>> the call of _Tesselation::VertexHandle _Tesselation::move(…) ?
>>
>> To encourage discussion I am making a small git commit with one line
>> changed ;)
>>
>> best regards
>> Janek Kozicki
>>
>> ___
>> Mailing list: https://launchpad.net/~yade-dev
>> Post to : yade-dev@lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~yade-dev
>> More help : https://help.launchpad.net/ListHelp
>>
>
___
Mailing list: https://launchpad.net/~yade-dev
Post to : yade-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~yade-dev
More help : https://help.launchpad.net/
Re: [Yade-dev] Yade is not compatible with CGAL_4.13
Hi Janek,
I think that was the right fix, thanks.
Note that yade is not using the periodic triangulation implemented in CGAL
[1] so there is no question on that point afaik.
Bruno
[1] that's because yade included periodicity years before cgal. I actually
implemented a periodicity based on cgal's non-periodic triangulation. Cgal
devs made periodic regular triangulation available more recently -
triggered by me for a part - but it's still not used in yade. Hopefully it
will be used one day but the refactoring it implies is a bit daunting.
On Fri, 4 Jan 2019 at 19:46, Janek Kozicki wrote:
> It appears that this function move_point(…) had long been deprecated.
> I guess that's got something to do with previous modifications where we
> had to use weighted points differently.
>
> I propose to modify this line into calling move(…) instead.
>
> This isn't change like the previous one where the C++ template
> structure was changed and I had to dwell deeply into code to recover
> the original implementation. This is a "small" change of the code
> which I did not write, so I suppose that Bruno I will need you to
> certify this.
>
> Let's see the code in old version:
> /usr/include/CGAL/Regular_triangulation_3.h
>
> #ifndef CGAL_NO_DEPRECATED_CODE
> template < class Gt, class Tds, class Lds >
> typename Delaunay_triangulation_3::Vertex_handle
> Delaunay_triangulation_3::
> move_point(Vertex_handle v, const Point & p)
> {
> CGAL_triangulation_precondition(! is_infinite(v));
> CGAL_triangulation_expensive_precondition(is_vertex(v));
>
> // Dummy implementation for a start.
>
> // Remember an incident vertex to restart
> // the point location after the removal.
> Cell_handle c = v->cell();
> Vertex_handle old_neighbor = c->vertex(c->index(v) == 0 ? 1 : 0);
> CGAL_triangulation_assertion(old_neighbor != v);
>
> remove(v);
>
> if (dimension() <= 0)
> return insert(p);
> return insert(p, old_neighbor->cell());
> }
> #endif
>
>
> And the code in new version:
> /usr/include/CGAL/Regular_triangulation_3.h
>
> template
> typename Regular_triangulation_3::Vertex_handle
> Regular_triangulation_3::
> move(Vertex_handle v, const Weighted_point& p)
> {
> CGAL_triangulation_precondition(!is_infinite(v));
> if(v->point() == p)
> return v;
>
> Self tmp;
> Vertex_remover remover(tmp);
> Vertex_inserter inserter(*this);
> return Tr_Base::move(v,p,remover,inserter);
> }
>
> Which calls tr_Base::move(…) from file:
> /usr/include/CGAL/Triangulation_3.h
>
> template < class Gt, class Tds, class Lds >
> template < class VertexRemover, class VertexInserter >
> typename Triangulation_3::Vertex_handle
> Triangulation_3::
> move(Vertex_handle v, const Point& p,
> VertexRemover& remover, VertexInserter& inserter)
> {
> CGAL_assertion(remover.hidden_points_begin() ==
> remover.hidden_points_end());
> CGAL_triangulation_precondition(!is_infinite(v));
>
> if(v->point() == p)
> return v;
>
> Vertex_handle w = move_if_no_collision(v,p,remover,inserter);
> if(w != v)
> {
> remove(v, remover);
> return w;
> }
>
> return v;
> }
>
> Also let's look at the documentation for move_point:
>
>
> https://doc.cgal.org/latest/Periodic_3_triangulation_3/classCGAL_1_1Periodic__3__Delaunay__triangulation__3.html#aa06932079eb7cf6eee8c5c9998088e34
>
> It says that it is just a little more optimized regular move. So it
> may be not surprise that those codes are not exactly the same. Maybe
> move_point became obsolete, because they removed the inefficiency
> that was present in older versions.
>
> Note that this function was NOT removed from periodic triangulations.
> This may be important since yade works in periodic too. So perhaps in
> Tesselation.ipp there should be a separate move(…) function for
> periodic cases, to use the old move_point function again when it is
> possible. Bruno?
>
> The compilation passed all tests and checks. But do those tests cover
> the call of _Tesselation::VertexHandle _Tesselation::move(…) ?
>
> To encourage discussion I am making a small git commit with one line
> changed ;)
>
> best regards
> Janek Kozicki
>
> ___
> Mailing list: https://launchpad.net/~yade-dev
> Post to : yade-dev@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~yade-dev
> More help : https://help.launchpad.net/ListHelp
>
___
Mailing list: https://launchpad.net/~yade-dev
Post to : yade-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~yade-dev
More help : https://help.launchpad.net/ListHelp
Re: [Yade-dev] Yade is not compatible with CGAL_4.13
It appears that this function move_point(…) had long been deprecated.
I guess that's got something to do with previous modifications where we
had to use weighted points differently.
I propose to modify this line into calling move(…) instead.
This isn't change like the previous one where the C++ template
structure was changed and I had to dwell deeply into code to recover
the original implementation. This is a "small" change of the code
which I did not write, so I suppose that Bruno I will need you to
certify this.
Let's see the code in old version:
/usr/include/CGAL/Regular_triangulation_3.h
#ifndef CGAL_NO_DEPRECATED_CODE
template < class Gt, class Tds, class Lds >
typename Delaunay_triangulation_3::Vertex_handle
Delaunay_triangulation_3::
move_point(Vertex_handle v, const Point & p)
{
CGAL_triangulation_precondition(! is_infinite(v));
CGAL_triangulation_expensive_precondition(is_vertex(v));
// Dummy implementation for a start.
// Remember an incident vertex to restart
// the point location after the removal.
Cell_handle c = v->cell();
Vertex_handle old_neighbor = c->vertex(c->index(v) == 0 ? 1 : 0);
CGAL_triangulation_assertion(old_neighbor != v);
remove(v);
if (dimension() <= 0)
return insert(p);
return insert(p, old_neighbor->cell());
}
#endif
And the code in new version:
/usr/include/CGAL/Regular_triangulation_3.h
template
typename Regular_triangulation_3::Vertex_handle
Regular_triangulation_3::
move(Vertex_handle v, const Weighted_point& p)
{
CGAL_triangulation_precondition(!is_infinite(v));
if(v->point() == p)
return v;
Self tmp;
Vertex_remover remover(tmp);
Vertex_inserter inserter(*this);
return Tr_Base::move(v,p,remover,inserter);
}
Which calls tr_Base::move(…) from file:
/usr/include/CGAL/Triangulation_3.h
template < class Gt, class Tds, class Lds >
template < class VertexRemover, class VertexInserter >
typename Triangulation_3::Vertex_handle
Triangulation_3::
move(Vertex_handle v, const Point& p,
VertexRemover& remover, VertexInserter& inserter)
{
CGAL_assertion(remover.hidden_points_begin() == remover.hidden_points_end());
CGAL_triangulation_precondition(!is_infinite(v));
if(v->point() == p)
return v;
Vertex_handle w = move_if_no_collision(v,p,remover,inserter);
if(w != v)
{
remove(v, remover);
return w;
}
return v;
}
Also let's look at the documentation for move_point:
https://doc.cgal.org/latest/Periodic_3_triangulation_3/classCGAL_1_1Periodic__3__Delaunay__triangulation__3.html#aa06932079eb7cf6eee8c5c9998088e34
It says that it is just a little more optimized regular move. So it
may be not surprise that those codes are not exactly the same. Maybe
move_point became obsolete, because they removed the inefficiency
that was present in older versions.
Note that this function was NOT removed from periodic triangulations.
This may be important since yade works in periodic too. So perhaps in
Tesselation.ipp there should be a separate move(…) function for
periodic cases, to use the old move_point function again when it is
possible. Bruno?
The compilation passed all tests and checks. But do those tests cover
the call of _Tesselation::VertexHandle _Tesselation::move(…) ?
To encourage discussion I am making a small git commit with one line changed ;)
best regards
Janek Kozicki
___
Mailing list: https://launchpad.net/~yade-dev
Post to : yade-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~yade-dev
More help : https://help.launchpad.net/ListHelp
Re: [Yade-dev] Yade is not compatible with CGAL_4.13
I'm looking into this.
On 4 Dec 2018, 23:42 +0100, Anton Gladky , wrote:
> Dear Yade developers,
>
> Yade fails to compile against newest CGAL_4,13,
> The corresponding bug on Debian tracker is [#911685].
>
> [#911685] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=911685
>
> Some relevant lines are here.
>
> ===
> CGT::SimpleVertexInfo, CGT::SimpleCellInfo>;
> CGT::_Tesselation::VertexHandle =
> CGAL::internal::CC_iterator CGAL::Triangulation_vertex_base_with_info_3 texInfo, CGAL::Epick, CGAL::Regular_triangulation_vertex_base_3 CGAL::Triangulation_ds_vertex_base_3 CGAL::Triangulation_vertex_base_with_info_
> 3 CGAL::Regular_triangulation_vertex_base_3 >,
> CGAL::Boolean_tag, CGAL::Boolean_tag >,
> CGAL::Alpha_shape_cell_base_3 CGAL::Triangulation_cell_base_with_info_3 ::SimpleCellInfo, CGAL::Epick,
> CGAL::Regular_triangulation_cell_base_3 >,
> CGAL::Boolean_tag, CGAL::Boolean_tag >, CGAL::Sequential_tag> >
> > >, CGAL::Boolean_tag, CGAL::Boolean_tag >, CGAL::Default,
> CGAL::Default, CGAL::Default>, false>; CGT::Real = double]':
> /root/yade/yade-2018.02b/pkg/dem/TesselationWrapper.cpp:186:28: required
> from here
> /root/yade/yade-2018.02b/lib/triangulation/Tesselation.ipp:78:12: error:
> 'CGT::_Tesselation CGT::SimpleCellInfo> >::RTriangulation' {aka 'class
> CGAL::Regular_triangulation_3 ::Triangulation_data_structure_3 CGAL::Triangulation_vertex_base_with_info_3 CGAL::Epick, CGAL::Regular_triangulation_vertex_base_3 >,
> CGAL::Boolean_tag e>, CGAL::Boolean_tag >, CGAL::Alpha_shape_cell_base_3 CGAL::Triangulation_cell_base_with_info_3 CGAL::Regular_triangulation_cell_base_3 >,
> CGAL::Boolean_tag, CGAL:
> :Boolean_tag >, CGAL::Sequential_tag>, CGAL::Default>'} has no member
> named 'move_point'; did you mean 'Bare_point'?
> Vh = Tri->move_point ( vertexHandles[id], Sphere ( Point ( x,y,z ),pow (
> rad,2 ) ) );
>
> ===
>
> It would be good if somebody tries to compile the yade
> against this CGAL version and fixes the issue. It is getting
> really painful that CGAL developers break the API permanently
>
> Thanks and regards
>
> Anton
> ___
> Mailing list: https://launchpad.net/~yade-dev
> Post to : yade-dev@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~yade-dev
> More help : https://help.launchpad.net/ListHelp
>
___
Mailing list: https://launchpad.net/~yade-dev
Post to : yade-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~yade-dev
More help : https://help.launchpad.net/ListHelp
[Yade-dev] Yade is not compatible with CGAL_4.13
Dear Yade developers,
Yade fails to compile against newest CGAL_4,13,
The corresponding bug on Debian tracker is [#911685].
[#911685] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=911685
Some relevant lines are here.
===
CGT::SimpleVertexInfo, CGT::SimpleCellInfo>;
CGT::_Tesselation::VertexHandle =
CGAL::internal::CC_iterator >,
CGAL::Boolean_tag, CGAL::Boolean_tag >,
CGAL::Alpha_shape_cell_base_3 >,
CGAL::Boolean_tag, CGAL::Boolean_tag >, CGAL::Sequential_tag>
> > >, CGAL::Boolean_tag, CGAL::Boolean_tag >, CGAL::Default,
CGAL::Default, CGAL::Default>, false>; CGT::Real = double]':
/root/yade/yade-2018.02b/pkg/dem/TesselationWrapper.cpp:186:28: required
from here
/root/yade/yade-2018.02b/lib/triangulation/Tesselation.ipp:78:12: error:
'CGT::_Tesselation >::RTriangulation' {aka 'class
CGAL::Regular_triangulation_3 >,
CGAL::Boolean_tag, CGAL::Boolean_tag >, CGAL::Alpha_shape_cell_base_3 >,
CGAL::Boolean_tag, CGAL:
:Boolean_tag >, CGAL::Sequential_tag>, CGAL::Default>'} has no
member named 'move_point'; did you mean 'Bare_point'?
Vh = Tri->move_point ( vertexHandles[id], Sphere ( Point ( x,y,z ),pow (
rad,2 ) ) );
===
It would be good if somebody tries to compile the yade
against this CGAL version and fixes the issue. It is getting
really painful that CGAL developers break the API permanently
Thanks and regards
Anton
___
Mailing list: https://launchpad.net/~yade-dev
Post to : yade-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~yade-dev
More help : https://help.launchpad.net/ListHelp
