trig functions

[nathanhack]

The usual suspects are really needed. cos, sin, tan, arccos, arcsin, arctan, and atan2.

[ericlagergren]

yep! they are on my TODO list. They likely will be implemented similarly to the Tan example; i.e., as continued fractions.

[nathanhack]

I basically have them already done, for my personal project. And if you'd be ok with it I could add them to the repo.

[ericlagergren]

I would love that. Le ven. 5 janv. 2018 à 18:20, nathanhack notifications@github.com a écrit :

I basically have them already done, for my personal project. And if you'd be ok with it I could add them to the repo if would like.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ericlagergren/decimal/issues/55#issuecomment-355716143, or mute the thread https://github.com/notifications/unsubscribe-auth/AFnwZ6uY3DgQHYwyEZ8SDc0D22eTbbK_ks5tHth5gaJpZM4RMW_1 .

[nathanhack]

Cool! Ok, I've started working on it. I should be able to put some cycles towards it this weekend and next. I maybe be close to done by end of next weekend.

[ericlagergren]

Awesome, I appreciate it. If it's any help, I've been using continued fractions from (https://doi.org/10.1007/978-1-4020-6949-9, but I have a .pdf copy I could "loan" you), but I'm sure there's plenty of other ways of doing it, too.

[ericlagergren]

Hey, just wondering how this is coming along. No pressure, of course—I appreciate the contribution. I'm just planning my schedule out and wanted to see if I could help at all.

[nathanhack]

I've completed non-concurrent versions of Sin, Cos, Tan, Asin, Acos, Atan (in the my fork of your project). I wanted to add Atan2 before I submit the PR. I do apologize, I meant to have completed this last weekend but I wasn't able to work on it. But I have a PR in draft and I'm thinking I'll be done with Atan2 later tonight and put in the PR tomorrow.

[ericlagergren]

@nathanhack you're totally fine, I'm not worried about time. I just wanted to touch base is all.

non-concurrent

Just fwiw, this is the default. The only concurrency promises the API makes is that z will be clobbered, but x, y, u, etc. are all "read only." Same as math/big.

[nathanhack]

Thanks for the clarification. That was the working assumption I made while coding. I also know that some of the biggest performance gains will require concurrent versions (that run with more than one go routine). But I don't have concurrent versions made and was making sure you knew that the ones I was proposing to add with the PR were also only intended for a single thread.

That said, since I'm near a PR, there are a few design choices I made that I should get your feedback on before I submit, to streamline the PR.

The first question is an important one, given the arguments in a signature Trig(z decimal.Big, theta decimal) ... I was working under the assumption that all precision decisions during the function would be based on the z variable rather than the theta variable. Thus, if z's context has a precision of 100 then result would have the same precision and all calculations are derived using that precision. Is that acceptable?

For the second question, I currently use the signature result, err := Trig(z *deciaml.Big , input ...) pattern for all the functions. However, I realize that I could just use NaN (signaling type with conditions) instead of returning an error which would make the usage a bit prettier, and less cumbersome result := Trig(z *deciaml.Big , input ...). It's your project, do you have a preference, or a requirement on the documentation page that I missed? On second thought, looks like you use the latter basically exclusively (except for the SetString()). So I'll use NaN too, unless you have a reason to not do the same.

[ericlagergren]

Gotcha. I should really write up a CONTIBUTING.md or something, it's kinda unfair to force people who want to help out with the library guess at what to do.

I was working under the assumption that all precision decisions during the function would be based on the z variable rather than the theta variable.

Yep, that's correct. At some point I'd love to have forms that take a decimal.Context, but I haven't come up with an API design I like quite yet. So, given

func F(z, x *decimal.Big) *decimal.Big

the result should be correctly rounded out to z.Context.Precision digits of precision (significant figures), using the decimal.ToNearestEven rounding mode. So that means...

and all calculations are derived using that precision

...this is slightly incorrect in that the routine must usually (internally) use a working precision that's higher than the desired result precision. (For example, see how Exp adjusts the working precision by 3 + k + 2 in exp.go.)

I currently use the signature result, err := Trig(z *deciaml.Big , input ...)

I would prefer setting the error on z and returning. I omitted (T, error) from the API because it's nice being able to chain calls, à la a.Add(b).Mul(c).Quo(d). There's no need to check if theta == nil—generally, the arguments are assumed to be non-nil unless otherwise specified (this follows the behavior seen in the Go stdlib).

I'm not an expert on trig functions, so I'm not sure what error conditions might occur, but typically if the result is undefined (like log(-1), for example) the result should be a quiet NaN with decimal.InvalidOperation set. (For example, see logSpecials in log.go.)

Normally I'd point you towards (http://speleotrove.com/decimal), but the GDA spec doesn't have trig functions so the best we can do is try to stick to what the spec does for similar functions.

[nathanhack]

Sweet, I think we're on the same page. Thanks for the quick response. I should be able to make the changes and have the PR in tomorrow.

[ericlagergren]

Cheers. Feel free to ping me if you've got any more questions. (I also edited my comment a couple times just now.)

[nathanhack]

and all calculations are derived using that precision

...this is slightly incorrect in that the routine must usually (internally) use a working precision that's higher than the desired result precision

Sorry I wasn't clear but that's what I meant by "derived."

I think we're good. Of course, I'm sure there will be other things that come up to discuss once the PR is issued.

[ericlagergren]

https://github.com/ericlagergren/decimal/commit/d837f4aaf1f70dfcdab00979d720c29732943b6e