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@lists.launchpad.net > 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) _______________________________________________ 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