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.

Sign up for GitHub

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

Jump to bottom

Scalar subtraction #270

Merged
merged 31 commits into from
May 1, 2024
Merged

Scalar subtraction #270

merged 31 commits into from
May 1, 2024

Conversation

jdcasale
Copy link
Contributor

@jdcasale jdcasale commented Apr 26, 2024 •
edited
Loading

Adds a scalar subtraction compute function for primitive arrays.

  • Bounds checking is done for integer values only on the minimum and maximum elements. (ie if neither the smallest nor the largest elementg underflow, nothing else will)
  • No bounds checking is performed for floating-point operations as they will over/underflow to inf

@jdcasale jdcasale marked this pull request as ready for review April 26, 2024 19:05
gatesn
gatesn reviewed Apr 29, 2024
View reviewed changes
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
@jdcasale jdcasale force-pushed the jc/ipc_take branch 2 times, most recently from b4e0987 to 15644d8 Compare April 29, 2024 16:39
Base automatically changed from jc/ipc_take to develop April 29, 2024 18:12
gatesn
gatesn reviewed Apr 30, 2024
View reviewed changes
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/benches/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/ptype.rs Outdated Show resolved Hide resolved
vortex-ipc/src/reader.rs Outdated Show resolved Hide resolved
gatesn
gatesn reviewed Apr 30, 2024
View reviewed changes
vortex-array/benches/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/array/chunked/compute/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/chunked/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/mod.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Outdated Show resolved Hide resolved
vortex-array/src/scalar/primitive.rs Outdated Show resolved Hide resolved
@jdcasale jdcasale marked this pull request as draft April 30, 2024 21:13
@jdcasale jdcasale marked this pull request as ready for review May 1, 2024 03:17
lwwmanning
lwwmanning reviewed May 1, 2024
View reviewed changes
vortex-array/src/array/primitive/compute/subtract_scalar.rs Outdated Show resolved Hide resolved
vortex-array/src/array/primitive/compute/subtract_scalar.rs Outdated Show resolved Hide resolved
vortex-array/src/compute/scalar_subtract.rs Show resolved Hide resolved
vortex-scalar/Cargo.toml Outdated Show resolved Hide resolved
vortex-scalar/src/primitive.rs Show resolved Hide resolved
vortex-dtype/src/ptype.rs
($self:expr, | $_:tt $enc:ident | $($body:tt)*) => ({
macro_rules! __with__ {( $_ $enc:ident ) => ( $($body)* )}
use $crate::PType;
use vortex_dtype::half::f16;
Copy link
Member

Choose a reason for hiding this comment

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

you might also want/need use half::f16 in case it's not imported in places this macro is used

Copy link
Contributor

Choose a reason for hiding this comment

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

The vortex_dtype::half is effectively a "vendored" version of half. This ensures we use consistent thing everywhere and also makes the implementation somewhat internal. As in, we could replace this for a different f16 crate.

Copy link
Member

Choose a reason for hiding this comment

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

ohhhh, I see, sure

@lwwmanning lwwmanning enabled auto-merge (squash) May 1, 2024 13:24
@lwwmanning lwwmanning merged commit 7c33ee3 into develop May 1, 2024
2 checks passed
@lwwmanning lwwmanning deleted the jc/scalar-subtract branch May 1, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gatesn gatesn gatesn left review comments

@lwwmanning lwwmanning lwwmanning approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jdcasale @lwwmanning @gatesn