search

KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
691 stars 229 forks source link

[Suggestion] Angle pseudostructure and more trig functions #1699

Open dewiniaid opened 8 years ago

dewiniaid commented 8 years ago

While KSP and kOS use degrees for everything, most published formulas for orbital mechanics use radians. In some cases (like the formula for eccentric anomaly to/from mean) has a significant impact in calculations -- both speed and precision loss due to constantly converting back and forth. (There's also the loss of code readability involved in making the conversions).

I propose a new Angle subclass of ScalarFloatValue that represents angles. It would know its current unit (primary representation), either degrees (default) or radians. (Or gradians if we really want to, but I doubt there's a practical use). The rest of this issue is long since it is more of a design proposal/spec than anything else.

New Suffixes

ScalarValue:degrees - Returns a new Angle representing this value in degrees

ScalarValue:radians - Returns a new Angle representing this value in radians.

Angle:degrees - (overrides ScalarValue:degrees) If this angle's current unit is degrees, return it as-is. Otherwise, return a new Angle that is the same as this angle but uses degrees as its primary form of representation.

Angle:radians - (overrides ScalarValue:radians) As above, but using radians as its unit.

Angle:units - Returns DEGREES or RADIANS, depending on the current unit.

Angle:circle - Returns the quantity of the current unit that makes up a circle. (360 or 2*pi)

Angle:sin, etc: - Alias to SIN(angle) (and other trigonometric functions)

Angle:scalar or Angle:float - Returns this angle as a ScalarFloatValue.

Angle:normalized (possibly a different name to not confuse with vectors) - Returns this angle as a value between 0 and 360 degrees or 0 to 2*pi radians. (e.g. 540:degrees:normalized = 180:degrees)

Operators

The general rule is:

  1. If multiple angles are present (e.g. binary operator with two angles), all angles are converted to the same units as the leftmost angle.
  2. If treating all angles as their parent classes for the operation would return a ScalarFloatValue, instead return that value as an angle in the same units as those used for the operation.

For purposes of examples, assume the following initialization:

LOCAL a1 IS 180:degrees.
LOCAL a2 IS constant:pi:radians.

Operators with two angles

Convert right operand to left operand's units, then do math/comparisons/etc as if ScalarValues, then return the angle in same units as the left operand.

Note this means addition and multiplication are potentially not commutative (deg+rad and rad+deg will return different results, but both represent the same angle).

PRINT a1+a2.  // pi in radians = 180 degrees, 180+180 = 360, returns 360:degrees.
PRINT a2+a1.  // returns (2*pi):radians

Operators with one angle and one ScalarValue

Assume that the ScalarValue is an angle in the same units as the angle.

PRINT a1+90.  // 270:degrees.
PRINT 1.5*a2.  // (1.5*pi):radians

All other two-argument operators.

Treat the angle as its ScalarFloatValue counterpart -- e.g. use the parent class's implementation.

PRINT V(1,0,0) * a1.  // V(180,0,0).

Functions

MOD: should function using the same rules for binary operators.
MOD(540:degrees, constant:pi:radians*2) = 180:degrees

Functions and suffixes that take an angle as input: (e.g. SIN,COS,TAN) Should convert the angle to the function's native units (I assume radians for the trig functions at leasta) first.

Functions and suffixes that return an angle: (e.g. ARCSIN/ARCCOS/ARCTAN):

Adding the complete suite of trigonometry functions (e.g. secant => SEC, cosecant => CSC, etc.) would be useful, as well as the hyperbolic trigonometry functions. (COSH, ARCOSH, etc.)

To avoid polluting the global namespace, these could be added as suffixes on Angle or on ScalarValue.

Impact on compatibility with existing scripts.

The general behavior of subclassing ScalarFloatValue should mean that existing scripts should mostly continue to function as-is, with a few possible exceptions:

Python has a mechanism from __future__ import *feature*. kOS already has a sort-of compiler directive (@ALLOWGLOBALS), so it could be possible to extend it with something like a @SUPPORTS directive.

The Python version, adapted to kOS nomenclature, looks like this:

In the case of this particular feature, only the backwards-incompatible parts would necessarily need to be disabled: namely the effects of functions that currently return a value in radians.

Dunbaratu commented 8 years ago
  • Scripts that use angle an identifier: Presumably these will work as-is for the same reason that "V" and "D" are valid variables and don't break the ability to make vectors and directions.

This is in fact a misfeature, because if you do a delegate variable like local V is foo@., according to scoping rules calling v()should give you the local delegate call and mask the built-in V() but it doesn't because when seeing a function call to something called v, we search for functions all the way up to global scope and only when we fail that do we then search for variables all the way up to global scope (so you get the global built-in V when you shouldn't). This is wrong and it's a thing we're trying to fix elsewhere.

Also, I do not like putting these :degrees and :radians suffixes onto every scalar there is, pretending all of them are Angles, even things that aren't, like velocity magnitudes, or throttle settings, or loop counters, etc.

If we were putting this to a vote, I'd vote very strongly against it.

zakukai commented 6 years ago

It would be nice if all the trig were simply based in radians - mathematically speaking it's a much better fit - but of course at this point that would be a very disruptive change.

My take on this would be to skip all the subclassing and auto-conversion stuff. Angle is simply a new struct which marks a (contained) scalar as representing some kind of angle. So Angle is not a scalar, but (like Vector or Direction) supports a similar set of operations.

In contrast with the original proposal, I'd say Angle should not be a subclass of Scalar, and Angle should never automatically convert to a scalar. It introduces too many potential problems. If someone wants to convert an Angle to a scalar, they can use a suffix:

set theta to 30:degrees. // theta is an Angle instance print theta:in_degrees. // theta:in_degrees is a scalar

Without the auto-conversion, however, it means that functions that we can't provide a truly smooth transition for functions that return an angle:

set theta to arcsin(0.5). // Presently, theta would be a scalar. Ideally it should be an Angle

The original proposal could get around this: arcsin() returns an Angle, but Angle is a subclass of Scalar, so if you pass the result to something expecting an angle, it'll work, and if you pass the result to something expecting a scalar, it might work (that is, you get the scalar component of the angle without the units) - but IMO this is inherently dangerous. Conversions should be explicit.

Unfortunately this means either change arcsin(), vectorangle(), etc. to yield Angles rather than scalars (which breaks compatibility with existing scripts) or introduce new, redundant versions of this functionality which yield Angles rather than scalar angles in degrees. I think going for the compatibility break would be the preferable approach, but I don't know how much trouble this would cause for people using KOS.

Functions that accept an angle can simply be made backward-compatible by treating scalars as angles-in-degrees: sin(30) → 0.5 // sin of a scalar treats the scalar as an angle in degrees. sin(constant:pi / 6) → 0.009 // Not what the programmer wanted, I'd wager, but the argument is a scalar, so we treat it as degrees! sin(constant:pi:radians / 6) → 0.5 // sin of an Angle

Keeping degrees as the standard default for trig functions is far from ideal IMO, but this way at least there is a way to perform trig without conversions involved.

I wouldn't extend this to addition, however: 30:degrees + 10 → error // When you add things, units have to match! Either convert 10 to an angle or convert 30:degrees to a scalar...

addition and division work as long as the result fits the supported types: 30:degrees / 3 → 10:degrees // No problem! 30 / 10:degrees → error // Result would be in units "per degree" which isn't supported *9:degrees 5:degrees** → error // Result would be in units "degrees squared" which isn't supported

I understand your concern about the :degrees and :radians suffixes to scalars described in the original post... But I think it makes some sense. It fits the way we unitize numbers in English ("five degrees" rather than "degrees 5"). And it doesn't really imply that all Scalars are a kind of Angle, but rather that all Scalars can be combined with a unit to create an Angle. In a programming sense it's something that should probably just be a constructor for Angle, but expressively it works well as a suffix for scalars.

(C++ handles this by allowing suffixes as part of the numeric literal syntax: so you can define a _radians suffix in your program and say "5_radians" as a convenient way of constructing an angle - it's a bit limited since you can't use the same suffix on a numeric variable, but it provides a convenient way of defining such conversions without having to centralize their declarations, or turn the intrinsic numeric types into "objects". It might make sense to do something similar with unit suffixes in KOS: at least as far as making the syntax for "unit suffixes" distinct from the syntax for "structure suffixes" so the former does not clutter up the latter.)

As you point out, for some scalars, such a conversion would be nonsensical. In many of these cases, that's true because the scalar in question already represents some physical quantity - it is itself a unitized quantity, but the units in question aren't encoded in the scalar holding the value. There are, therefore, all sorts of nonsensical operations you can perform with these numbers: like adding a temperature to a velocity magnitude, or taking the square root of a distance. The language at present doesn't attempt to stop you from doing nonsensical things.

I think moving toward having a unit system would be a good thing in general, not just for angles. (But providing this for angles at least might be a nice way out of this degrees/radians issue... For most units in the game, the units used by the game are acceptable to the game's audience - people are used to seeing their ship's velocity in m/s, etc. - but in the case of angles, people are very accustomed to degrees, but the math really naturally favors radians...) It does complicate things a bit (esp. if these concepts are extended to vectors, etc.) and it can't protect from all possible errors obviously, but it helps.

MaraRinn commented 5 years ago

This kind of "units as a class" approach will also help remedy issues such as using altitude instead of radius when calculating SMA.

To handle set theta to arcsin(0.5) while being backwards compatible we can just use constructors: set theta to AngleFromDegrees(arcsin(0.5)), or use new functions such as set theta to ArcSinAngle(0.5).

Then overloads such as multiplication will exist so angles can be multiplied by angles, but addition can be denied by not providing the overload to allow Angle to be added to anything other than Angle.

But this still won't solve the original problem: when attempting to implement Kepler's equation of M = E - e sin E, E will be an Angle, e sin E will be a scalar, and the programmer is left wondering how to coerce e sin E into being an angle. So they try AngleFromDegrees(e * sin(E)) and we're back where we started, a programmer trying to implement the formula they found in Wikipedia and not understanding how it was derived, or that the units are all explicitly radians.

But to go ahead with the feature, we'd end up with constructors and overrides for operations:

Thus what I currently write as

set M to E  - e * sin(E) * constant:RadToDeg.

Could become:

set M to E - AngleFromRadians(e * sin(E)).
set M to AngleFromRadians(E:radians - e * sin(E)).
set M:radians to E:radians - e * sin(E).

There would be no "Angle:scalar" because scalar is already returned by Angle:degrees or Angle:radians. There could be a constant CircleAngle which is an Angle type and thus can be used as CircleAngle:degrees or CircleAngle:radians to provide the respective value (360 or 2π respectively).

Thus restating the original post:

LOCAL a1 = AngleFromDegrees(180).
LOCAL a2 = AngleFromRadians(constants:Pi).

PRINT a1 + a2. // output will be "Angle #FFA45" or whatever, since print takes a scalar

Angles will never allow coercion into scalar, and only support multiplication (and division) by scalar, and addition to an Angle.

The modulus operator would not need to be changed, programmers will just need to be specific about what they're trying to do:

set A to AngleFromDegrees(540).
set A:degrees to mod(A:degrees, 360).

Noting of course that 540º is not the same as 180º when it comes to plotting a Hohmann Transfer!

nuggreat commented 5 years ago

I would not like seeing an angle class being added to kOS for several reasons first because it would break backwards compatibility, second if you are not coming from a programing background a person would not have any idea what a class is and likely end up very confused, third there are places where the a angle class would be a pain to work with.

Personally I would prefer a simpler solution more along the lines of what calculators do when you are calculating in degrees or radians namely by using a flag you can set to change between the radian and degree calculation modes as needed by the script.

As I envision it such a flag would be set on the CORE using the suffix TRIGMODE looking and accepting ether "RAD", "RADIAN", "DEG", or "DEGREE" as it's values and in code would end up looking like SET CORE:TRIGMODE TO "RAD". the reason for adding the mode switch to the core is to prevent more than one core from interfering with each other if they are bolt doing math and loaded at the same time like what might happen with a flag global to the mod. Said flag would also revert to degrees when the core is reloaded for any reason ie scene load or reboot.

The functions effected by said TRIGMODE flag would be the functions you use to do math ie the SIN, COS, TAN, ARCSIN, ARCCOS, ARCTAN, ARCTAN2, and VANG, as well as the functions used to make DIRECTIONS that take degrees as one or more of there arguments ie the R, HEADING, and ANGLEAXIS functions.

There are several useful properties of such a flag over using a class.

First this will preserve backwards compatibility by adding the ability to change modes while keeping the default to what it was in the past.

Second the more limited scope of the proposed will not require changing every place in the mod where degrees or radians are used.

While there are a few notable disadvantages to making an angle class

First such a new class is likely to break backwards compatibility and I for one don't wan to re-wright all of my scripts to use this new class.

Second classes as a concept are not necessarily intuitive to a person new to programing in general and while there are only a few people who do try to pick up kOS as there first programing language such people do exist.

Third there exist return in kOS like ANGULARVEL or ANGULARMOMENTUM which return a vector with a magnitude in radians that would not be immediately converted into the a new class when you are working with them and thus likely cause at least some problems when working with them.

MaraRinn commented 5 years ago

First such a new class is likely to break backwards compatibility and I for one don't wan to re-wright all of my scripts to use this new class.

They won't break anything if the regular trig functions are left alone. As per my suggestion the new Angle-ified trig functions would explicitly take or return angles, such as SinAngle, ArcSinAngle.

AngularVel and AngularMomentum return vectors, and people who don't know what the magnitude represents are going to learn eventually anyway. Those vectors are only used by people who know what they represent, and there's no need to change one of the dimensions of the vectors to a class. They'll either be converted using vec:mag * Rad2Deg, or AngleFromRadians(vec:mag).

Using a flag to modify existing trig functions will lead to interesting scenarios like using a library that handles angles in radians, then having all your degree-based maths go bizarrely wrong. We could apply hacks such as adding a flag to every scalar to indicate what trig mode was in use when the value was assigned, then convert on the fly if the mode is different. But that will get ugly quickly and introduce all kinds of bugs that will catch people by surprise.

For the people that want to try something new, the Angle class would be there for them when they need it.

nuggreat commented 5 years ago

For anvularVel and angularMomentum don't assume a person knows what they are when they start messing about with them i didn't when i started playing about with them.

Using a flag to modify the trig functions will only cause problems when the lib doesn't reset the trig mode on exit of a function just before the return and at least to me that tend to fall under make your lib in such a way it doesn't impact the rest of your code save for how you want it to. Admittedly yes someone who just copied a lib from the internet might get unexpected results but we already have such unexpected results so it is only slightly different than what it is now

Also the inability to add a scalar to a angle would cause problems with at least some scripts I have seen including at least one of mine mostly related to work with latitude and longitude stuff