Re: [Yade-dev] Shop::aabbExtrema() : from boost::python::tuple return type to std::vector ?

2018-08-13 Thread Jerome Duriez 
See this commit. 



Regarding your suggestions, Bruno :

" - dereferencing body pointer without checking it"
should be corrected now [*]

"- dynamic cast"
Are you referring to the Shape cast [**] ? If yes, I understand it as a 
good thing to check whether we indeed have a sphere and did not touch it.


"- only works for spheres"
Yes, at least it is clearly stated in the doc.. I'm actually not aware 
of a more generic extrema function somewhere else...



Still open to further changes obviously, thanks,

Jérôme

[*] 
https://github.com/yade/trunk/blob/1db13fb1183b9e294dc9761da76cfa4fc2791cc1/pkg/dem/Shop_02.cpp#L879
[**] 
https://github.com/yade/trunk/blob/1db13fb1183b9e294dc9761da76cfa4fc2791cc1/pkg/dem/Shop_02.cpp#L880


--
Chargé de Recherche / Research Associate
Irstea, RECOVER
3275 route de Cezanne – CS 40061 13182 Aix-en-Provence Cedex 5 FRANCE
+33 (0)4 42 66 99 21

On 08/08/2018 15:54, Bruno Chareyre wrote:

Hi Jérôme,
This change would be sensible, seems to me (especially regarding your 
second bullet point).

Note that pair would make more sense than a vector<>.

I would point out that the implementation of this function looks 
sloppy in various other ways:

- dereferencing body pointer without checking it
- dynamic cast
- only works for spheres

Isn't there a more generic extrema function somewhere? I'm wondering.

Bruno



On 1 August 2018 at 14:29, Jerome Duriez > wrote:


Hi again,

I would like changing in pkg/dem/Shop*pp the py::tuple
aabbExtrema() [*] to std::vectoraabbExtrema() (keeping
obviously the Python exposure).


I think this would

- make the Shop C++ files one (small) step closer from the ideal
situation discussed at
https://www.mail-archive.com/yade-dev@lists.launchpad.net/msg13308.html


- thus avoiding the back and forth Python-C++ boost conversions in
all C++ uses of aabbExtrema() e.g. [**]

- help me solving easily
https://bugs.launchpad.net/yade/+bug/1764424
 ;-) (thanks Jan for
suggestion)

- change (I guess, after py/wrapper/customConverters.cpp) from the
user's side the aabbExtrema() value (after typed in YADE terminal)
from a tuple to a list...


Thoughts ?

Jérôme

[*]
https://github.com/yade/trunk/blob/master/pkg/dem/Shop.hpp#L165

and
https://github.com/yade/trunk/blob/master/pkg/dem/Shop_02.cpp#L874

[**]
https://github.com/yade/trunk/blob/master/pkg/dem/Shop_02.cpp#L352

and L353

--
Chargé de Recherche / Research Associate
Irstea, RECOVER
3275 route de Cezanne – CS 40061 13182 Aix-en-Provence Cedex 5 FRANCE
+33 (0)4 42 66 99 21


___
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



___
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] Shop::aabbExtrema() : from boost::python::tuple return type to std::vector ?

2018-08-08 Thread Bruno Chareyre 
Hi Jérôme,
This change would be sensible, seems to me (especially regarding your
second bullet point).
Note that pair would make more sense than a vector<>.

I would point out that the implementation of this function looks sloppy in
various other ways:
- dereferencing body pointer without checking it
- dynamic cast
- only works for spheres

Isn't there a more generic extrema function somewhere? I'm wondering.

Bruno



On 1 August 2018 at 14:29, Jerome Duriez  wrote:

> Hi again,
>
> I would like changing in pkg/dem/Shop*pp the py::tuple aabbExtrema() [*]
> to std::vectoraabbExtrema() (keeping obviously the Python
> exposure).
>
>
> I think this would
>
> - make the Shop C++ files one (small) step closer from the ideal situation
> discussed at https://www.mail-archive.com/yade-dev@lists.launchpad.net/ms
> g13308.html
>
> - thus avoiding the back and forth Python-C++ boost conversions in all C++
> uses of aabbExtrema() e.g. [**]
>
> - help me solving easily https://bugs.launchpad.net/yade/+bug/1764424 ;-)
> (thanks Jan for suggestion)
>
> - change (I guess, after py/wrapper/customConverters.cpp) from the user's
> side the aabbExtrema() value (after typed in YADE terminal) from a tuple to
> a list...
>
>
> Thoughts ?
>
> Jérôme
>
> [*] https://github.com/yade/trunk/blob/master/pkg/dem/Shop.hpp#L165 and
> https://github.com/yade/trunk/blob/master/pkg/dem/Shop_02.cpp#L874
> [**] https://github.com/yade/trunk/blob/master/pkg/dem/Shop_02.cpp#L352
> and L353
>
> --
> Chargé de Recherche / Research Associate
> Irstea, RECOVER
> 3275 route de Cezanne – CS 40061 13182 Aix-en-Provence Cedex 5 FRANCE
> +33 (0)4 42 66 99 21
>
>
> ___
> 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] Shop::aabbExtrema() : from boost::python::tuple return type to std::vector ?

2018-08-01 Thread Jerome Duriez 

Hi again,

I would like changing in pkg/dem/Shop*pp the py::tuple aabbExtrema() [*] 
to std::vectoraabbExtrema() (keeping obviously the Python 
exposure).



I think this would

- make the Shop C++ files one (small) step closer from the ideal 
situation discussed at 
https://www.mail-archive.com/yade-dev@lists.launchpad.net/msg13308.html


- thus avoiding the back and forth Python-C++ boost conversions in all 
C++ uses of aabbExtrema() e.g. [**]


- help me solving easily https://bugs.launchpad.net/yade/+bug/1764424 
;-) (thanks Jan for suggestion)


- change (I guess, after py/wrapper/customConverters.cpp) from the 
user's side the aabbExtrema() value (after typed in YADE terminal) from 
a tuple to a list...



Thoughts ?

Jérôme

[*] https://github.com/yade/trunk/blob/master/pkg/dem/Shop.hpp#L165 and 
https://github.com/yade/trunk/blob/master/pkg/dem/Shop_02.cpp#L874
[**] https://github.com/yade/trunk/blob/master/pkg/dem/Shop_02.cpp#L352 
and L353


--
Chargé de Recherche / Research Associate
Irstea, RECOVER
3275 route de Cezanne – CS 40061 13182 Aix-en-Provence Cedex 5 FRANCE
+33 (0)4 42 66 99 21


___
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