savvy enum as input

[daniellga]

Hi! In the example below HDataType is just an enum created with [#savvy] attribute. HArray is a savvy struct that accepts &HDataType as an argument. When I pass dtype like below, it gives an error. I have to use $.ptr to make it work. When I read the guide I understood the first call to new_from_values should work. What am I doing wrong?

> dtype = HDataType$float32()
> ptr = dtype$.ptr
> HArray$new_from_values(as.array(c(1,2,3)), dtype)
Error: Unexpected type: Expected externalptr, got environment
> HArray$new_from_values(as.array(c(1,2,3)), ptr)
HArray
Float32
[1, 2, 3]

Here's what appears in the wrapper file:

HDataType <- new.env(parent = emptyenv())
HDataType$float32 <- function() {
  .savvy_wrap_HDataType(.Call(HDataType_float32__impl))
}

HDataType$float64 <- function() {
  .savvy_wrap_HDataType(.Call(HDataType_float64__impl))
}

HDataType$complex32 <- function() {
  .savvy_wrap_HDataType(.Call(HDataType_complex32__impl))
}

HDataType$complex64 <- function() {
  .savvy_wrap_HDataType(.Call(HDataType_complex64__impl))
}

.savvy_wrap_HDataType <- function(ptr) {
  e <- new.env(parent = emptyenv())
  e$.ptr <- ptr
  e$print <- HDataType_print(ptr)
  e$eq <- HDataType_eq(ptr)
  e$ne <- HDataType_ne(ptr)

  class(e) <- "HDataType"
  e
}

HArray declaration:

#[savvy]
impl HArray {
    fn new_from_values(arr: Sexp, dtype: &HDataType) -> savvy::Result<HArray> {
...

HDataType declaration:

#[derive(PartialEq)]
pub enum HDataType {
    Float32,
    Float64,
    Complex32,
    Complex64,
}

#[savvy]
impl HDataType {
    fn float32() -> Self {
        Self::Float32
    }
...

[yutannihilation]

I don't understand what's happening, but #[savvy] only supports function and struct, not enum.

[daniellga]

oh, didn't know that... But it seems it parsed everything right and it even works when I use $.ptr...

[yutannihilation]

Can you show the result of savvy-cli --version?

[daniellga]

savvy-cli 0.4.0

[yutannihilation]

Could you try 0.4.1? While I'm still not confident that enum is safely supported, I don't see any problem on my local (I just tried quickly, so I might be wrong.)

[daniellga]

now it works gracefully, thanks! Just a point, if I use HArray$new_from_values(c(1,2,3)), dtype) I get:

Note the only difference from the right code was the use of an atomic vector instead of an array. I will check my side of the code but, as far as I remember, it should panic.

#[derive(Clone)]
pub struct HArray(pub Arc<dyn HArrayR>);

#[savvy]
impl HArray {
    fn new_from_values(arr: Sexp, dtype: &HDataType) -> savvy::Result<HArray> {
        if let Some(dim) = arr.get_dim() {
            let mut dim: Vec<usize> = dim.iter().map(|z| *z as usize).collect();
            dim.reverse();

            match (arr.into_typed(), dtype) {
                (TypedSexp::Real(arr), HDataType::Float32) => {
                    dim.reverse();
                    let slice: &[f64] = arr.as_slice();
                    let v: Vec<f32> = slice.iter().map(|x| *x as f32).collect();
                    let harray = harmonium_core::array::HArray::new_from_shape_vec(dim, v).unwrap();
                    let data = Arc::new(harray);
                    Ok(HArray(data))
                }
                (TypedSexp::Real(arr), HDataType::Float64) => {
                    let v: Vec<f64> = arr.as_slice().to_vec();
                    let harray = harmonium_core::array::HArray::new_from_shape_vec(dim, v).unwrap();
                    let data = Arc::new(harray);
                    Ok(HArray(data))
                }
                (TypedSexp::Complex(arr), HDataType::Complex32) => {
                    let slice: &[Complex<f64>] = arr.as_slice();
                    let v: Vec<Complex<f32>> = slice
                        .iter()
                        .map(|z| Complex::new(z.re as f32, z.im as f32))
                        .collect();
                    let harray = harmonium_core::array::HArray::new_from_shape_vec(dim, v).unwrap();
                    let data = Arc::new(harray);
                    Ok(HArray(data))
                }
                (TypedSexp::Complex(arr), HDataType::Complex64) => {
                    let v: Vec<Complex<f64>> = arr.as_slice().to_vec();
                    let harray = harmonium_core::array::HArray::new_from_shape_vec(dim, v).unwrap();
                    let data = Arc::new(harray);
                    Ok(HArray(data))
                }
                _ => panic!("not valid input types"),
            }
        } else {
            panic!("arr must be of array type.");
        }
    }
}

[yutannihilation]

Oh, thanks for your attempt! I think you can change the last lines to the following. Unlike extendr, panic! means the session crash in savvy.

                _ => Err("not valid input types".into()),
            }
        } else {
            Err("arr must be of array type.".into())
        }

[yutannihilation]

My guess is that c(1,2,3) doesn't have dim attribute so you'll get the "arr must be array type" error.

[daniellga]

Ok! I thought it would panic. I will later implement my errors anyway... My plan is to return an externalpointer to an enum representing my errors.

[yutannihilation]

Thanks. Good to know enum can work without extra effort!

Btw, in this case, do you want something simple like this rather than the function form of Float32 etc?

HDataType <- list(
    Float32 = 1L
    Float6 = 2L,
    Complex32 = 3L
    Complex64 = 4L
)

[daniellga]

I don't get it, do you mean using a list instead of an environment?

[yutannihilation]

I'm wondering if this is nicer

HArray$new_from_values(as.array(c(1,2,3)), HDataType$float32)

than this

HArray$new_from_values(as.array(c(1,2,3)), HDataType$float32())

Also, would it be nice if you could avoid implementing getters on Rust

[daniellga]

I actually do this on my zzz.R file:

.onLoad <- function(libname, pkgname){
  # Assure only one instance of HDataType's external pointer is created for each enum variant.
  HDataType$float32 = HDataType$float32()
  HDataType$float64 = HDataType$float64()
  HDataType$complex32 = HDataType$complex32()
  HDataType$complex64 = HDataType$complex64()
  lockEnvironment(HDataType, bindings = TRUE)
}

The reason I do this is to oblige the user to use one of the options provided. I brought the idea from torch, I think...

[yutannihilation]

Thanks, let me think if it can be supported nicer.

[yutannihilation]

PyO3's case. Supporting only fieldless enums is a fair decision. I think I will follow.

https://pyo3.rs/v0.20.0/class.html?highlight=enum#pyclass-enums https://pyo3.rs/v0.20.0/conversions/traits.html?highlight=enum#extract-and-the-frompyobject-trait

[daniellga]

That's fine for me... My use case for them seems to be exactly what is documented there.