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

atan #207

Merged
merged 20 commits into from
Dec 17, 2020
Merged

atan #207

merged 20 commits into from
Dec 17, 2020

Conversation

matthuszagh
Copy link
Contributor

Adds 2-argument arctangent function. Parameters and result are i32 integers for fast computation. Only the 16 MSBs of the inputs are used (16 LSBs are discarded). The x and y inputs can range from -1 to 1, which corresponds to i32::MIN and i32::MAX, respectively. The output ranges from -pi to pi, which corresponds to i32::MIN and i32::MAX, respectively.

Related

@matthuszagh matthuszagh changed the title Atan i32 atan Dec 17, 2020
dsp/src/iir.rs Show resolved Hide resolved
dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated
/// The angle between the x-axis and the ray to the point (x,y). The
/// result range is from i32::MIN to i32::MAX, where i32::MIN
/// corresponds to an angle of -pi and i32::MAX corresponds to an
/// angle of +pi.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that really correct? i32::MAX != -i32::MIN.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's actually i32::MIN == -pi and at the same time also i32::MIN == pi. Then i32::MAX == pi*(1 - 1/(1 << 31))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right that makes sense. It's also worth noting that i32::MIN is not actually reachable. I can get i32::MAX with atan2(0, i32::MIN). The closest results are 2147461318 and -2147461318 with atan2(1 << 16, i32::MIN)) and atan2(-1 << 16, i32::MIN)).

Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit.
Looks good!

dsp/src/trig.rs Outdated Show resolved Hide resolved
dsp/src/trig.rs Outdated
/// The angle between the x-axis and the ray to the point (x,y). The
/// result range is from i32::MIN to i32::MAX, where i32::MIN
/// corresponds to an angle of -pi and i32::MAX corresponds to an
/// angle of +pi.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's actually i32::MIN == -pi and at the same time also i32::MIN == pi. Then i32::MAX == pi*(1 - 1/(1 << 31))

@jordens
Copy link
Member

jordens commented Dec 17, 2020

Bors r+

bors bot added a commit that referenced this pull request Dec 17, 2020
207: atan r=jordens a=matthuszagh

Adds 2-argument arctangent function. Parameters and result are `i32` integers for fast computation. Only the 16 MSBs of the inputs are used (16 LSBs are discarded). The x and y inputs can range from -1 to 1, which corresponds to `i32::MIN` and `i32::MAX`, respectively. The output ranges from -pi to pi, which corresponds to `i32::MIN` and `i32::MAX`, respectively.

- [godbolt](https://rust.godbolt.org/z/nahKrT)

# Related
- #206

Co-authored-by: Matt Huszagh <huszaghmatt@gmail.com>
@bors
Copy link
Contributor

bors bot commented Dec 17, 2020

Canceled.

@jordens
Copy link
Member

jordens commented Dec 17, 2020

Bors r+

@bors
Copy link
Contributor

bors bot commented Dec 17, 2020

Build succeeded:

@bors bors bot merged commit 2f122d1 into quartiq:master Dec 17, 2020
@matthuszagh matthuszagh deleted the atan-i32 branch December 17, 2020 23:23
@matthuszagh matthuszagh mentioned this pull request Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants