yade-dev team mailing list archive

["Janek Kozicki (yade)" <jkozicki-yade@xxxxxxxxx>]

Hi Gaël,

>     p₃ = 1 - p₂ - p₃                          (1c)

If p₃ was previously uninitialized (or zero). Then this equation must
be incorrect by definition, it uses p₃ on right side.
Ah, but in your photos [1] you wrote p₃ = 1 - p₁ - p₂. :-)

> I also think I might need the confirmation that the fix I propose
> indeed is relevant and safe to be implemented.

The PFacet author should comment on that :)

> Final point: although the changes to be made are small, I would really
> like to submit it personnaly to the git repository so that I learn how
> it is done.

Looks like you already have an account on gitlab: https://gitlab.com/GLorieul
And also you are a member of yade-dev on gitlab:
https://gitlab.com/yade-dev/trunk/-/project_members

This means that you can open a merge request right away. For some
introduction you can have a look at: https://yade-dem.org/doc/gitrepo.html

But in short opening a merge request is very easy: you git push your
stuff as a branch (non-master branch, that is). For example if you
have your commits in master, this should work (but I type from memory
now):

  git checkout -b gaelfix # to create ("-b") a branch (gaelfix) for you
  git push --set-upstream origin gaelfix # to push it

this command should print (among other messages) a http link to open
a merge request. Just open this link and then in the web browser you
can write some description and then click submit. And that's it :)

Klaus was doing recently the same thing :) I think you two guys could
also open another merge request to update this file gitrepo.html:

https://gitlab.com/yade-dev/trunk/-/blob/master/doc/sphinx/gitrepo.rst

best regards
Janek

PS: nice calculations btw: [1] http://dl.free.fr/mbGj9Moc0 :))


Gaël Lorieul said:     (by the date of Tue, 02 Mar 2021 16:31:44 -0300)

> Hello all,
> 
> I think I fixed issue 694548.
> 
> Contrary to my initial belief, it was not related to what I had
> observed for gridConnection objects (this will be the subject of a
> different thread). Instead, it was a bug regarding the calculation of
> weights. I'll show that in the case of the MWE posted by Jeffrey
> Knowles (issue 694548's original poster).
> 
> The motion of PFacet is dictated by that of the 3 gridNode objects it
> connects. Each gridNode recieves a different force value calculated
> from the force of the interaction. The function
> `Law2_ScGridCoGeom_FrictPhys_CundallStrack::go`
> (`pkg/common/Grid.cpp:537`) does that using weights:
> ```
> scene->forces.addForce(geom->id3, geom->weight[0] * -force);
> scene->forces.addForce(geom->id4, geom->weight[1] * -force);
> scene->forces.addForce(geom->id5, geom->weight[2] * -force);
> ```
> 
> Those weights are calculated in `pkg/common/PFacet.cpp`
> (`Ig2_Sphere_PFacet_ScGridCoGeom::projection()`) as
> 
>          ‖v₁‖²(v₀·v₂) - (v₁·v₂)(v₀·v₁)
>     p₁ = —————————————————————————————        (1a)
>              ‖v₀‖²‖v₁‖² - (v₀·v₁)²
>          ‖v₀‖²(v₁·v₂) - (v₀·v₂)(v₀·v₁)
>     p₂ = —————————————————————————————        (1b)
>              ‖v₀‖²‖v₁‖² - (v₀·v₁)²
>     p₃ = 1 - p₂ - p₃                          (1c)
> 
> But actually, it should be:
>          ‖v₁‖²(v₀·v₂) - (v₁·v₂)(v₀·v₁)
>     p₂ = —————————————————————————————        (2a)
>              ‖v₀‖²‖v₁‖² - (v₀·v₁)²
>          ‖v₀‖²(v₁·v₂) - (v₀·v₂)(v₀·v₁)
>     p₃ = —————————————————————————————        (2b)
>              ‖v₀‖²‖v₁‖² - (v₀·v₁)²
>     p₁ = 1 - p₂ - p₃                          (2c)
> 
> Hence the bug (MWE "passes" after making the permutation).
> 
> In the end, it is only a permutation of p₁,p₂ and p₃. However, let me
> stress that the fix I propose is not just a "hack" (me randomly
> interchanging p1,p2,p3 until I get the result I expected). Instead, I
> did the calculations from scratch again, and the equations (2) are the
> result of that. The scans of my calculations are available here [1]. If
> need be, I can make a LaTeX transcript of those calculations.
> 
> However, it uneases me that the consequences of this bug can be seen
> fairly easily, and most certainly the original author of the code would
> have noticed it straight away. Especially since the rest of the
> function seems well written. Hence perhaps things are more complicated
> than that? Could I be breaking something else? To make sure of that, I
> attempted to compile the first version of the code to feature this
> calculation (commit dfec202), but unfortunately without success 🙁️
> (and I don't really want to spend more than the 2.5 days I already
> spent just to try to compile (old) stuff). Note that commit dfec202 is
> the commit that introduced PFacet objects in Yade.
> 
> Final point: although the changes to be made are small, I would really
> like to submit it personnaly to the git repository so that I learn how
> it is done. I would need a bit of guidance for that though 😉️. I also
> think I might need the confirmation that the fix I propose indeed is
> relevant and safe to be implemented.
> 
> Gaël
> 
> [1] http://dl.free.fr/mbGj9Moc0
> _______________________________________________
> Mailing list: https://launchpad.net/~yade-dev
> Post to     : yade-dev@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~yade-dev
> More help   : https://help.launchpad.net/ListHelp


-- 
--
Janek Kozicki, PhD. DSc. Arch. Assoc. Prof.
Gdańsk University of Technology
Faculty of Applied Physics and Mathematics
Department of Theoretical Physics and Quantum Information
--
http://yade-dem.org/
http://pg.edu.pl/jkozicki (click English flag on top right)