Expression parser broken for Python objects

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Expression parser broken for Python objects

Post by uwestoehr »

I encountered today that the expression parser does not handle quantities correctly. It simply ignores the unit voncersion factors set in the units scheme.

I spent a lot of time to find the problem but I am stuck. Maybe someone can help?:

- take current master and this simple simulation document:
Expression-Bugtest.FCStd
(178.04 KiB) Downloaded 25 times
- give the object named "GiveMeAnExpression!" and expression, for example "1+2". It does not matter if you set the expression via the constraint dialog or the property editor.

result:
FreeCAD_lWmTxhyZ7E.gif
FreeCAD_lWmTxhyZ7E.gif (95.46 KiB) Viewed 1226 times
So the result 3e-6 V not 3 V. So the factors we set in UnitsSchemaInternal.cpp are simply ignored by the expression parser.

The same problem occurs for any other unit, but only in task panels written in Python.

Does anybody has an idea how to tell the Expression parser about our conversion factors?
chrisb
Veteran
Posts: 53920
Joined: Tue Mar 17, 2015 9:14 am

Re: Expression parser broken for Python objects

Post by chrisb »

Confirmed. You should add your FreeCAD infos too.

Code: Select all

OS: macOS 10.16
Word size of FreeCAD: 64-bit
Version: 0.21.29997 (Git)
Build type: Release
Branch: master
Hash: b52967d52ac46eff7c59e74d991f3f5b298944ef
Python 3.10.5, Qt 5.15.4, Coin 4.0.0, Vtk 9.1.0, OCC 7.6.2
Locale: C/Default (C)
Installed mods: 
  * FC_SU
  * FeedsAndSpeeds 0.4.0
  * fcgear 1.0.0
  * fasteners 0.3.51
  * sheetmetal 0.2.56
  * ExplodedAssembly
  * Curves 0.5.4
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
chrisb
Veteran
Posts: 53920
Joined: Tue Mar 17, 2015 9:14 am

Re: Expression parser broken for Python objects

Post by chrisb »

The issue exists also in 0.20:

Code: Select all

OS: macOS 10.16
Word size of FreeCAD: 64-bit
Version: 0.20.29177 (Git)
Build type: Release
Branch: (HEAD detached at 0.20)
Hash: 68e337670e227889217652ddac593c93b5e8dc94
Python 3.9.13, Qt 5.12.9, Coin 4.0.0, Vtk 9.1.0, OCC 7.5.3
Locale: C/Default (C)
Installed mods: 
  * FC_SU
  * FeedsAndSpeeds 0.4.0
  * fcgear 1.0.0
  * fasteners 0.3.51
  * sheetmetal 0.2.56
  * ExplodedAssembly
  * Curves 0.5.4
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
chrisb
Veteran
Posts: 53920
Joined: Tue Mar 17, 2015 9:14 am

Re: Expression parser broken for Python objects

Post by chrisb »

As a workaround you can enter the values with units:

Code: Select all

1V+2V
As you most probably don't have a constant expression you can add a unit to a unitless value like this

Code: Select all

<<someExpression>>*1V + <<someOtherExpression>>*1V
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Expression parser broken for Python objects

Post by wmayer »

I encountered today that the expression parser does not handle quantities correctly.
It has nothing to do with the expression parser. It determines the two NumberExpressions "1" and "2" and the OperatorExpression "+" and computes the correct NumberExpression "3".

The further handling of the NumberExpression happens outside the context of the expression parser.
It simply ignores the unit voncersion factors set in the units scheme.
No, it doesn't. It's your expression that is to blame because you expect the unit V but write a dimensionless expression. Because the spin box expects a quantity with the unit V it recreates a new quantity with the internal value 3 and the unit V.

Now V expressed in SI base units gives: 1V = 1 kg * m² / (A * s³)
But because lengths internally are measured in mm the above expression becomes: 10⁶ * kg * mm² / (A * s³)

So, the internal value of 1 V is 10⁶ and consequently if the internal value is 3 the actual quantity is 3 * 10⁻⁶ V or 0.003 mV

If you want expressions to behave as expected it's important to write them correctly.
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: Expression parser broken for Python objects

Post by uwestoehr »

wmayer wrote: Mon Aug 15, 2022 6:43 am No, it doesn't. It's your expression that is to blame because you expect the unit V but write a dimensionless expression.
But isn't this the idea of having set a unit? I mean the property is an App::PropertyPotential. The dialog tells the user the user also that the unit is "V". So as user I expect when I enter a "1" the result will be "1 V". As user it totally confusing that then I set an expression, e.g. because the potential should be half of a length, you take a unitless value from a spreadsheet to calculate the potential.
wmayer wrote: Mon Aug 15, 2022 6:43 am But because lengths internally are measured in mm the above expression becomes: 10⁶ * kg * mm² / (A * s³)
That is what I mean. How should the user know this? Entering just entering "3" gives you "3 mV" entering 3 as expression gives you "0.003 mV". But as user I expect:
- to get the same result
- the result is in V

Is there anything we can do to improve the user experience? I fear that when I as experienced user struggle with this, others will do as well.
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: Expression parser broken for Python objects

Post by uwestoehr »

The situation for the user is even more confusing:

- take for example the initial pressure constraint, its unit is App::PropertyPressure, thus the unit is Pa
- entering "1" leads to "1000":
FreeCAD_0rO9Nvk6HW.png
FreeCAD_0rO9Nvk6HW.png (8.74 KiB) Viewed 1093 times

Sure, the internal length of mm triggers this, but how can the user know? Therefore I think we should do something.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Expression parser broken for Python objects

Post by wmayer »

The dialog tells the user the user also that the unit is "V". So as user I expect when I enter a "1" the result will be "1 V".
And if the original value was 1 mV and you entered the expression 1 + 2 do you then expect 3mV or 3V?

If a dimensionless value was entered directly into a spin box then it can validate the user input by appending the last valid unit string (including a prefix like mV) and then it parses the new input string again.

However, for expressions this validation step is not done on purpose because expressions are evaluated when their value is needed and therefore it's irrelevant what value is shown in a spin box. If an incomplete expression were validated then it would cause a situation where the spin box shows a value that the user may expect but this value wouldn't be used for the calculation but the result value of the expression.
As user it totally confusing that then I set an expression, e.g. because the potential should be half of a length, you take a unitless value from a spreadsheet to calculate the potential.
If you omit important information in expressions you cannot really expect that it then computes the output you want to have. As a scientist you should be able to write correct expressions.
Entering just entering "3" gives you "3 mV" entering 3 as expression gives you "0.003 mV". But as user I expect:
- to get the same result
- the result is in V
As explained above the expression engine bypasses all the logic implemented in spin boxes or other control widgets.
Is there anything we can do to improve the user experience?
Yes, avoid dimensionless quantities where they are inappropriate.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Expression parser broken for Python objects

Post by wmayer »

The situation for the user is even more confusing:
There is nothing that is more confusing:
1 Pa = kg / (m * s²) = 10⁻³ kg / (mm * s²)
<=>
1 kg / (mm * s²) = 1000 Pa
Therefore I think we should do something.
Maybe it should be disallowed to write dimensionless expressions when a unit is required. Or at least inside the dialog a warning could be printed.
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: Expression parser broken for Python objects

Post by uwestoehr »

wmayer wrote: Mon Aug 15, 2022 12:06 pm
The dialog tells the user the user also that the unit is "V". So as user I expect when I enter a "1" the result will be "1 V".
And if the original value was 1 mV and you entered the expression 1 + 2 do you then expect 3mV or 3V?
As you can see in my screencast the original value was "1 V" not "1 mv".
wmayer wrote: Mon Aug 15, 2022 12:06 pm There is nothing that is more confusing:
1 Pa = kg / (m * s²) = 10⁻³ kg / (mm * s²)
Do we really expect that users?:
- know we handle the length unit internally in mm
- what a unit consists of (for example when you ask me what V consists of, I cannot answer and I use it daily :oops: )
- that because of our mm handling they have to check in what order the length appears in a unit and then to recalculate

I use several laboratory software and when there is an edit field for voltage I can just e.g. write in "3.3" and get 3.3 V.

In my opinion a proper solution would be that the expression is evaluated as it is already. If the result is unitless, the the unit of the QuantitySpinBox is added to the result. This way the user does not need to know our internals. He knows the unit for pressure is Pa and whatever he enter is then in Pa.
Post Reply