PR #2475: Expression syntax extension

Merged, abandoned or rejected pull requests are moved here to clear the main Pull Requests forum.
wmayer
Site Admin
Posts: 15716
Joined: Thu Feb 19, 2009 10:32 am

Re: PR #2475: Expression syntax extension

Post by wmayer » Sat Sep 28, 2019 3:02 pm

The branch is merged now. But:
  1. number_power_handler for MatrixPy, PlacementPy and RotationPy is wrong
  2. VectorPy::number_multiply_handler has been extended by MatrixPy, RotationPy and PlacementPy but the implementation doesn't make sense to me and from a math point of view it is wrong
To 1:

Code: Select all

m=App.Matrix()
m.scale(2,2,2)
m**1 # => Matrix ((4,0,0,0),(0,4,0,0),(0,0,4,0),(0,0,0,1))
To 2:
It's possible to multiply a vector on the left with a matrix on the right. But therefore the vector must be a row vector and the output will be again a row vector. But the implementation returns a scaled matrix which is quite strange.

For PlacementPy and RotationPy a vector is returned but the implementation does exactly the same as when multiplying a rotation/placement on the left with a vector on the right. Thus it leads to some confusion because people may assume that matrix multiplication is commutative which in general it is not.

As said above when multiplying a vector v with a matrix M then we need a row-vector v^T, i.e. its transformed and afterwards must transpose the result again to get a column vector. This means we do:
(v^T * M)^T <==> M^T * v^T^T <==> M^T * v

OpenInventor's class SbMatrix has the methods multMatrixVec and multVecMatrix and for the latter it also uses the transposed matrix for the multiplication.
realthunder
Posts: 1536
Joined: Tue Jan 03, 2017 10:55 am

Re: PR #2475: Expression syntax extension

Post by realthunder » Sun Sep 29, 2019 1:24 am

wmayer wrote:
Sat Sep 28, 2019 3:02 pm
[*]number_power_handler for MatrixPy, PlacementPy and RotationPy is wrong
Eh... don't know what I am thinking at the time...

[*]VectorPy::number_multiply_handler has been extended by MatrixPy, RotationPy and PlacementPy but the implementation doesn't make sense to me and from a math point of view it is wrong
I was trying to expose matrix scale by vector somehow, and wasn't treating rotation and placement as matrix. But yes, it does cause confusion. I'll remove that from VectorPy, and add an expression function for matrix scale instead.
Try Assembly3 (latest version 0.11) along with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
Zolko
Posts: 842
Joined: Mon Dec 17, 2018 10:02 am

Re: PR #2475: Expression syntax extension

Post by Zolko » Mon Sep 30, 2019 7:35 am

realthunder wrote:
Mon Sep 02, 2019 1:21 pm
Well, I can add number protocol to Placement too, just like matrix, so you can do something like

Code: Select all

Group.<<Box.>>.Placement * Group.<<$Cube.>>.Placement ^ -1
wmayer wrote:
Sat Sep 28, 2019 3:02 pm
The branch is merged now. But: number_power_handler for MatrixPy, PlacementPy and RotationPy is wrong
Nice, I'll try as soon as I can get hold of an AppImage. BUT: to be honest, I don't completely understand the purpose of a general power mechanism for Placements (or even matrices). While it's possible to define that:

Code: Select all

Placement.inverse() = Placement^-1
This is true for numbers (1/X = X^-1), but there is a rigorous mathematical notion behind it, that allows to define X^-3 or X^(1/2), but for matrices or Placements, what would mean :

Code: Select all

Pacement^2
or
Placement^(1/2)
Is there any other use-case than combination or inverse for a Placement ? And even the operator multiplication is a convention because it's not the matrix-multiplication of the 4x4 matrix representation of a Placement. There is a precise definition and use-case for Placement.multiply() and Placement.inverse():

Code: Select all

Group.<<Box.>>.Placement.multiply( Group.<<$Cube.>>.Placement.inverse() )
but what is the precise definition and use-case for Placement.power() (Placement^N or Placement**N) ? Since Placement.multiply() and Placement.inverse() are already defined and coded in Base::Placement, wouldn't it be better to have a pass-through for these 2 functionalities — and only these two — in the ExpressionEngine parser, rather than define a whole new set of mathematical functions with uncertain theoretical background ? What are the use-cases for Placement**5 or Placement^-2 ?
try the Assembly4 workbench for FreCAD v0.19
install with Tools > Addon Manager > Assembly4 — tutorials here and here
realthunder
Posts: 1536
Joined: Tue Jan 03, 2017 10:55 am

Re: PR #2475: Expression syntax extension

Post by realthunder » Mon Sep 30, 2019 7:50 am

Zolko wrote:
Mon Sep 30, 2019 7:35 am
but what is the precise definition and use-case for Placement.power() (Placement^N or Placement**N) ? Since Placement.multiply() and Placement.inverse() are already defined and coded in Base::Placement, wouldn't it be better to have a pass-through for these 2 functionalities in the ExpressionEngine parser, rather than define a whole new set of mathematical functions with uncertain theoretical background ? What are the use-cases for Placement**5 or Placement^-2 ?
I understand you argument here, which is why I added a built-in function called 'minvert()' in this PR. You can use this function to invert matrix, placement and rotation. You can checkout the test case here to find out various usage examples.

I didn't add the python function calling syntax because it requires almost complete rewrite of the current expression syntax rules (which I've already done it in one of my branch). But it does not add much functionality if using whitelist, while blacklist may expose too much security risk.

Anyway, I think a built-in function is enough for your use case here. I may consider merging the syntax change if it is really necessary.
Try Assembly3 (latest version 0.11) along with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
Zolko
Posts: 842
Joined: Mon Dec 17, 2018 10:02 am

Re: PR #2475: Expression syntax extension

Post by Zolko » Mon Sep 30, 2019 8:14 am

realthunder wrote:
Mon Sep 30, 2019 7:50 am
I didn't add the python function calling syntax because it requires almost complete rewrite of the current expression syntax rules (which I've already done it in one of my branch)
yes I know, and I very much liked that functionality: it was very natural to use. Could you please point to the source-code where this implementation was done ? (in the -asm3 or -LinkStage3 branch ?)
try the Assembly4 workbench for FreCAD v0.19
install with Tools > Addon Manager > Assembly4 — tutorials here and here
realthunder
Posts: 1536
Joined: Tue Jan 03, 2017 10:55 am

Re: PR #2475: Expression syntax extension

Post by realthunder » Mon Sep 30, 2019 8:39 am

Zolko wrote:
Mon Sep 30, 2019 8:14 am
realthunder wrote:
Mon Sep 30, 2019 7:50 am
I didn't add the python function calling syntax because it requires almost complete rewrite of the current expression syntax rules (which I've already done it in one of my branch)
yes I know, and I very much liked that functionality: it was very natural to use. Could you please point to the source-code where this implementation was done ? (in the -asm3 or -LinkStage3 branch ?)
The syntax rule is defined here. You can compare it with the same file in upstream to see the big difference. This is the bison source file for generating the parser. The interpreter is implemented by extending the existing expression class with various new subclasses (header, source). You can find a high level overview of the changes here.
Try Assembly3 (latest version 0.11) along with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
Zolko
Posts: 842
Joined: Mon Dec 17, 2018 10:02 am

Re: PR #2475: Expression syntax extension

Post by Zolko » Mon Sep 30, 2019 9:05 am

whouaaahhh ... this is way beyond my programming skills. I'll trust you to come up with a good solution, thanks.
try the Assembly4 workbench for FreCAD v0.19
install with Tools > Addon Manager > Assembly4 — tutorials here and here
wmayer
Site Admin
Posts: 15716
Joined: Thu Feb 19, 2009 10:32 am

Re: PR #2475: Expression syntax extension

Post by wmayer » Mon Sep 30, 2019 12:22 pm

This is true for numbers (1/X = X^-1), but there is a rigorous mathematical notion behind it, that allows to define X^-3 or X^(1/2), but for matrices or Placements, what would mean :
On Wikipedia you can find some information for square roots of a matrix but it's only well-defined for positive-definite matrices while for arbitrary matrices it causes ambiguities. But what when using 1/3 as exponent instead? Here I couldn't find a valid definition. So, a power function for arbitrary exponents makes probably not much sense (especially not in a CAD program) and it should be limited to integers only as done by the current implementation.
And even the operator multiplication is a convention because it's not the matrix-multiplication of the 4x4 matrix representation of a Placement.
A placement consists of a rotational part and a translational part, i.e. its matrix representation is a matrix where its 3x3 sub-matrix is orthonormal and the fourth column is the translation vector. In that case the multiplication operator should exactly do the same for a placement or a matrix and even the inverse of both should give the same results.
What are the use-cases for Placement**5 or Placement^-2 ?
Maybe less for Placements but for Rotations it could be useful. If you have a given rotation vector and angle then e.g.
Rotation(vector, angle/5)**5 is the same as Rotation(vector, angle). Geometrically this means that instead of rotating around a vector by an angle in one go you split it into five steps.

Here the implementation of RotationPy::number_power_handler is not very efficient because it performs several quaternion multiplications instead of directly changing the angle only.
User avatar
Zolko
Posts: 842
Joined: Mon Dec 17, 2018 10:02 am

Re: PR #2475: Expression syntax extension

Post by Zolko » Mon Sep 30, 2019 12:42 pm

wmayer wrote:
Mon Sep 30, 2019 12:22 pm
And even the operator multiplication is a convention because it's not the matrix-multiplication of the 4x4 matrix representation of a Placement.
A placement consists of a rotational part and a translational part, i.e. its matrix representation is a matrix where its 3x3 sub-matrix is orthonormal and the fourth column is the translation vector. In that case the multiplication operator should exactly do the same for a placement or a matrix
yes, true, I hadn't though that enough

... and even the inverse of both should give the same results.
errrr ... I don't think so. If the rotation part is an orthonormal 3x3 matrix, then the inverse is the transpose, but the inverse of the translation is the opposite of the translation vector on column 4, whereas the transpose would put it on line 4 and with the same sign.

Therefore, all-in-all, the multiplication of Placements can be treated the same as for matrices, and it makes much sense to treat them identically as a generic matrix multiplication.

BUT the power() part seems much more questionable to me.
try the Assembly4 workbench for FreCAD v0.19
install with Tools > Addon Manager > Assembly4 — tutorials here and here
wmayer
Site Admin
Posts: 15716
Joined: Thu Feb 19, 2009 10:32 am

Re: PR #2475: Expression syntax extension

Post by wmayer » Mon Sep 30, 2019 2:07 pm

Zolko wrote:
Mon Sep 30, 2019 12:42 pm
errrr ... I don't think so. If the rotation part is an orthonormal 3x3 matrix, then the inverse is the transpose, but the inverse of the translation is the opposite of the translation vector on column 4, whereas the transpose would put it on line 4 and with the same sign.
As shown here the inverted placement of
y = R * x + t is x = Q * y - Q * t where Q is the transposed and inverse of R

Now make the block matrices (R|t) and (Q|-Q*t) and imagine that for both matrices the fourth row is (0, 0, 0, 1). I claim that (Q|-Q*t) is the inverse of (R|t) and to show that this is true the product of them must be I, the identity matrix.

(R|t) * (Q|-Q*t) because the fourth row of both is (0, 0, 0, 1) it's quite obvious that the 3x3 sub-matrix is R * Q = I and that the fourth row is again (0, 0, 0, 1). The only thing to check further is the fourth column. But when looking at this you will see it is:
R * (-Q * t) + t
=
(R * -Q) * t + t
=
- (R * Q) * t + t
=
- I * t + t
=
(0,0,0)
Post Reply

Who is online

Users browsing this forum: No registered users and 1 guest