New data generation functions and Neg trait implementation #94

9prady9 · 2016-12-15T07:09:27Z

Fixes #93
Fixes #89

function to create an array with same shape and type as input array filled with a constant

pavanky · 2016-12-15T07:50:07Z

Are there non templated versions of random number generators ?

jramapuram

Looks good. Just the one query.

jramapuram · 2016-12-15T12:27:16Z

src/arith/mod.rs

+    type Output = Array;
+
+    fn neg(self) -> Self::Output {
+        constant_like(0.0, &self) - self


Is there a benefit to this vs the following?

af::mul(&-1.0, &self);

Both are doing same number of operations (create a constant array and then do arithmetic operation of two arrays) under the hood except that the intrinsic operation is subtraction in the trait implementation where as the other version is doing multiplication. I think they would perform about the same - compiler may even optimise it for this particular cases (Multiplication by -1 and Subtraction from 0).

jramapuram · 2016-12-15T12:27:51Z

src/data/mod.rs

+/// # Return Values
+///
+/// Array with given constant value and input Array's shape and similar internal data type.
+pub fn constant_like(value: f64, input: &Array) -> Array {


Nice addition

9prady9 · 2016-12-17T17:06:57Z

@arrayfire/rust-devel Please check the latest commit, it should cover all the missing gaps now.

jramapuram · 2016-12-20T09:51:21Z

src/data/mod.rs

+/// # Return Values
+///
+/// Array filled with `value` and `dims` shape.
+pub fn create_constant_i64(value: i64, dims: Dim4) -> Array {


I see why you made these separate from the create_constant [precision], however can we do something to merge them? I.e. maybe an Enum or something? Not too big a deal either way though.

@jramapuram How can we do this using enum ?

I was thinking of something like this: https://is.gd/taX9ac

jramapuram · 2016-12-20T09:53:53Z

src/data/mod.rs

+/// # Return Values
+///
+/// Array of `dims` shape and filed with given constant `value`.
+pub fn create_constant(real: f64, imag: f64, dims: Dim4, dtype: DType) -> Array {


Is it possible to remove all the create_ parts from these functions? I think it is obvious that someone wants to create a constant if they call the constant function. It will also be more more consistent with the AF api

We can't have same function names. If we remove the create_ prefix then name clash will occur.

Ah yes forgot we already have that and need to specialize. See my gist for a possible solution: https://is.gd/taX9ac

jramapuram · 2016-12-20T09:54:11Z

src/data/mod.rs

+/// # Return Values
+///
+/// Array filled with `value` and `dims` shape.
+pub fn create_constant_u64(value: u64, dims: Dim4) -> Array {


Same comment w.r.t. create_

jramapuram · 2016-12-20T09:54:20Z

src/data/mod.rs

+/// # Return Values
+/// Array
+#[allow(unused_mut)]
+pub fn create_range(dims: Dim4, seq_dim: i32, dtype: DType) -> Array {


Same comment w.r.t. create_

jramapuram · 2016-12-20T09:54:28Z

src/data/mod.rs

+///
+/// Identity matrix
+#[allow(unused_mut)]
+pub fn create_identity(dims: Dim4, dtype: DType) -> Array {


Same comment w.r.t. create_

jramapuram · 2016-12-20T09:54:30Z

src/data/mod.rs

+///
+/// Array
+#[allow(unused_mut)]
+pub fn create_iota(dims: Dim4, tdims: Dim4, dtype: DType) -> Array {


Same comment w.r.t. create_

jramapuram · 2016-12-20T09:58:13Z

I found the need for constant a while ago. But since I don't really need U64_MAX & I64_MAX I approximated it with the following [using conv crate]:

use af;
use conv::{ConvUtil, Saturate};

/// Helper to return a constant value based on type
pub fn constant(dims: Dim4, aftype: DType, val: f32) -> Array {
  match aftype
  {
    DType::F32 => af::constant(val, dims),
    DType::F64 => af::constant(val.approx_as::<f64>().unwrap(), dims),
    DType::C32 => af::constant(Complex::new(val, 0f32)
                                , dims),
    DType::C64 => af::constant(Complex::new(val.approx_as::<f64>().unwrap(), 0f64)
                        , dims),
    DType::B8  => {
      if val > 0f32 {
        af::constant(true, dims)
      }else{
        af::constant(false, dims)
      }
    },
    DType::S32 => af::constant(val.approx_as::<i32>().saturate().unwrap(), dims),
    DType::U32 => af::constant(val.approx_as::<u32>().saturate().unwrap(), dims),
    DType::U8  => af::constant(val.approx_as::<u8>().saturate().unwrap(), dims),
    DType::S64 => af::constant(val.approx_as::<i64>().saturate().unwrap(), dims),
    DType::U64 => af::constant(val.approx_as::<u64>().saturate().unwrap(), dims),
    DType::S16 => af::constant(val.approx_as::<i16>().saturate().unwrap(), dims),
    DType::U16 => af::constant(val.approx_as::<u16>().saturate().unwrap(), dims),
  }
}

9prady9 · 2016-12-20T10:10:28Z

@jramapuram I don't like the create_ prefix either, but i couldn't figure out a way to keep both earlier versions of the data generation functions and the new ones with same function names. Even if i mark a function as deprecated, the new function cannot have the same name.

9prady9 · 2017-01-03T13:16:51Z

@pavanky @jramapuram Feedback pls, i want to release 3.4.1 of this wrapper this week if possible.

pavanky · 2017-01-03T13:31:00Z

@jramapuram @9prady9

why won't the function shown here work:
#94 (comment)

9prady9 · 2017-01-03T14:02:14Z

That version is sacrificing accuracy of u64 and i64 by accepting the constant value as an f32

pub fn constant(dims: Dim4, aftype: DType, val: f32)

I don't think we can do that as a library.

Edited: Added to that, we can't have another function with same name as constant, hence the type suffixes.

pavanky · 2017-01-03T14:05:54Z

@9prady9 accept f64 and have special version called constant_long or something. This is what we do for the C and C++ versions anyway.

9prady9 · 2017-01-03T14:08:04Z

I didn't check C++ API, but C API of arrayfire has different functions for u64, i64 and rest of the types.

Edited:

af_constant_ulong
af_constant_long
af_constant_complex and af_constant for test of the types

pavanky · 2017-01-03T14:13:24Z

@9prady9 I wasn't talking about having multiple functions, but about the name (specifically the create_ parts). Will @jramapuram's code causing a conflict?

9prady9 · 2017-01-03T14:15:13Z

Yes, we can't have same names.

9prady9 · 2017-01-03T14:15:41Z

For him it is working because he had them in different namespace.

jramapuram · 2017-01-03T15:25:19Z

Sorry for the delay. Infrequent internet over the winter break. Will review in 2 days.

9prady9 · 2017-01-09T04:30:19Z

@arrayfire/rust-devel Awaiting feedback.

jramapuram · 2017-01-09T13:05:36Z

Added a simple way to use Enums in comments. Another possible (but have not tested myself) way is to use generics: http://stackoverflow.com/questions/26810793/how-can-i-create-an-is-prime-function-that-is-generic-over-various-integer-types

See solution in that answer as well as comment in accepted answer. Not sure if the generics are robust enough to accept all numeric types yet though.

* constant_t * range_t * range_t * iota_t

9prady9 · 2017-01-10T16:11:52Z

@jramapuram please check now. Using your suggestion, i have consolidated all the separate create_constant_* functions into a single function constant_t with the suffix _t suggesting that it takes additional type parameter (that's the shortest name i could come up with that is different from constant).

@pavanky please check now. I have renamed all create_* fns to have _t suffix now.

pavanky · 2017-01-10T16:30:28Z

looks good 👍

jramapuram · 2017-01-10T16:36:42Z

Looks great!

9prady9 added 2 commits December 15, 2016 12:30

new data generation function: constant_like

bcd9909

function to create an array with same shape and type as input array filled with a constant

Implement negation trait for Array

63293af

9prady9 added the Feature label Dec 15, 2016

9prady9 added this to the 3.4.1 milestone Dec 15, 2016

This was referenced Dec 15, 2016

Creating arrays similar to other arrays #93

Closed

Implement Neg overloaded operator for Array #89

Closed

9prady9 requested review from jramapuram and pavanky December 15, 2016 07:13

jramapuram reviewed Dec 15, 2016

View reviewed changes

9prady9 changed the title ~~Constant_like and Neg trait~~ New data generation functions and Neg trait implementation Dec 18, 2016

9prady9 requested a review from jramapuram December 19, 2016 07:57

jramapuram requested changes Dec 20, 2016

View reviewed changes

9prady9 removed the request for review from pavanky December 20, 2016 10:06

9prady9 requested review from jramapuram and pavanky December 21, 2016 14:20

New fns to create arrays using Dim4 and DType params

10a20d1

* constant_t * range_t * range_t * iota_t

9prady9 force-pushed the constant_like_and_Negtrait branch from d202256 to 10a20d1 Compare January 10, 2017 16:09

jramapuram approved these changes Jan 10, 2017

View reviewed changes

9prady9 merged commit 78a0353 into arrayfire:devel Jan 10, 2017

9prady9 deleted the constant_like_and_Negtrait branch January 10, 2017 16:37

New data generation functions and Neg trait implementation #94

New data generation functions and Neg trait implementation #94

Uh oh!

Conversation

9prady9 commented Dec 15, 2016

Uh oh!

pavanky commented Dec 15, 2016

Uh oh!

jramapuram left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

9prady9 commented Dec 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

9prady9 Dec 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jramapuram commented Dec 20, 2016

Uh oh!

9prady9 commented Dec 20, 2016

Uh oh!

9prady9 commented Jan 3, 2017

Uh oh!

pavanky commented Jan 3, 2017

Uh oh!

9prady9 commented Jan 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pavanky commented Jan 3, 2017

Uh oh!

9prady9 commented Jan 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pavanky commented Jan 3, 2017

Uh oh!

9prady9 commented Jan 3, 2017

Uh oh!

9prady9 commented Jan 3, 2017

Uh oh!

jramapuram commented Jan 3, 2017

Uh oh!

9prady9 commented Jan 9, 2017

Uh oh!

jramapuram commented Jan 9, 2017

Uh oh!

9prady9 commented Jan 10, 2017

Uh oh!

pavanky commented Jan 10, 2017

Uh oh!

jramapuram commented Jan 10, 2017

Uh oh!

Uh oh!

9prady9 Dec 20, 2016 •

edited

Loading

9prady9 commented Jan 3, 2017 •

edited

Loading

9prady9 commented Jan 3, 2017 •

edited

Loading