Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trignometric function: choose the default unit (degrees, radians or gradians) #446

Closed
KevinAlvesGroupeBRIAND opened this issue Jul 21, 2021 · 3 comments
Assignees
Labels
rejected Rejected features

Comments

@KevinAlvesGroupeBRIAND
Copy link

KevinAlvesGroupeBRIAND commented Jul 21, 2021

Hi,


Describe the bug
I have a different result with a same expression due to a version incompatibility.

v. 3.7.3 => "cos(50)" return "0,96496602849..." (with processor.Parameters.AngleMeasurement = AngleMeasurement.Radian)
v. 4.0.1 => "cos(50)" return "0,64278760968..." (cannot specify a unit)

I choose to set all trigonometric function are in radians, and I don't want unit specified in the expression.


In version 3.7.3
In version 3.7.3, we could choose the unit of trigonometric functions (cos, sin, etc.) by default with the following code:

Processor processor = new Processor();
var expression = "cos(50)"; //example, with no unit specified in expression.
processor.Parameters.AngleMeasurement = AngleMeasurement.Radian; // "cos(50)" set to radian
var result = processor.Solve(expression); // return "0,96496602849..."

In version 4.0.1
I cannot specified the default unit for trigonometric function. In this version, the expression cos(50) is to degree.

Processor processor = new Processor();
var expression = "cos(50)"; //example, with no unit specified in expression.
//processor.Parameters.AngleMeasurement = AngleMeasurement.Radian;  // don't exist in 4.0.1
var result = processor.Solve(expression); // return "0,64278760968..."

Expected behavior
Trignometric function (cos, sin, tan, acos, asin, atan, etc.): choice of the default unit when the expression does not mention it, as :

processor.Parameters.AngleMeasurement = AngleMeasurement.Radian; //or AngleMeasurement.Degree / AngleMeasurement.Gradian

You can set "AngleMeasurement.Degree" when processor.Parameters.AngleMeasurement value is not specified (null value).

Actual behavior
The trignometric function is to degree and I cannot choose the default unit.


See also: https://github.com/sys27/xFunc/wiki/Breaking-Changes

The AngleMeasurement property is removed from ExpressionParameters, this feature is replaced by number units, which you can specify alongside a number, like 90 degree (see #268).


I am available if need more details.

Regards,

@sys27 sys27 self-assigned this Jul 24, 2021
@sys27
Copy link
Owner

sys27 commented Jul 24, 2021

Hi,

The removing of AngleMeasurement was intentional. Initially, units feature was implemented for static analysis (simplifier) to make expression more descriptive, consistent and help optimize it based on unit (now, units aren't a part of "runtime", but "parse" time). I chose degrees as a default unit and decided to remove AngleMeasurement feature, because it won't allow you to do static analysis (we don't know unit at "parse" time, so we don't know a "meaning" of expression, we can't change it because, there is a high change to break this expression) and you always can explicitly specify your unit.

Why don't/can't you specify your units in expression (like: cos(50 rad)? I didn't get why do you still need this feature from your description.

@KevinAlvesGroupeBRIAND
Copy link
Author

Hi,

It is true that I did not give the reason for the choice of the unit (radian)

Originally, in our case, the use of calculation expression was based on Excel.
The trigonometric functions were expressed to radians.

image

Documentation for Excel (cos function) : https://support.microsoft.com/en-us/office/cos-function-0fb808a5-95d6-4553-8148-22aebdce5f05

The indication of a unit in an expression is not desired because we try to keep the same notation as Excel as much as possible.

However, I understand that adding units directly to expressions is also a good idea.
But unfortunately, that does not really suit us for the use we want to make.

@sys27
Copy link
Owner

sys27 commented Jul 29, 2021

Unfortunately, I don't see any good solution here.

  • Return parameter to ExpressionParameters. I definitely don't want to do it.
  • Introduce default units to parser, which will allow us to change defaults for the entire parser instance. This feature will require changes to trigonometric functions, because the default degree unit is hardcoded there right now. So, basically it will need to restrict trigonometric function only to angle units (remove support for plain numbers) and rely on parser to "convert" numbers to angles (according default parameter). I'm not sure that I want to do it and commit to this feature.
  • Use any workaround. Excel and xFunc allows you to create user defined functions, which can smooth out differences between these apps. For example, xFunc supports functions to convert between different units, you can create the same functions in Excel, or any other approach, which solves the problem.

@sys27 sys27 added the proposal Features that are not approved yet label Jul 29, 2021
@sys27 sys27 added rejected Rejected features and removed proposal Features that are not approved yet labels Aug 8, 2023
@sys27 sys27 closed this as not planned Won't fix, can't repro, duplicate, stale Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rejected Rejected features
Projects
None yet
Development

No branches or pull requests

2 participants