You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/09/26 01:24:10 UTC
[GitHub] [incubator-tvm] mwillsey opened a new pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
mwillsey opened a new pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563
This PR packages up some varied work that @jroesch, @gussmith23, and I have been working on to improve the Rust bindings.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] jroesch commented on pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#issuecomment-701718674
We can put them here: https://github.com/apache/incubator-tvm/issues/6604.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] jroesch commented on pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#issuecomment-703849631
We need to update the docker images to match MxNet see #6628
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] jroesch commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497170872
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -184,28 +246,19 @@ impl NDArray {
/// let ctx = Context::cpu(0);
/// let mut ndarray = NDArray::empty(&mut shape, ctx, DataType::from_str("int32").unwrap());
/// ndarray.copy_from_buffer(&mut data);
- /// assert_eq!(ndarray.shape(), Some(&mut shape[..]));
+ /// assert_eq!(ndarray.shape(), shape);
/// assert_eq!(ndarray.to_vec::<i32>().unwrap(), data);
/// ```
pub fn to_vec<T>(&self) -> Result<Vec<T>, NDArrayError> {
- if !self.shape().is_some() {
- return Err(NDArrayError::EmptyArray);
- }
- let earr = NDArray::empty(
- self.shape().ok_or(NDArrayError::MissingShape)?,
- Context::cpu(0),
- self.dtype(),
- );
- let target = self.copy_to_ndarray(earr)?;
- let arr = target.as_dltensor();
- let sz = self.size().ok_or(NDArrayError::MissingShape)?;
- let mut v: Vec<T> = Vec::with_capacity(sz * mem::size_of::<T>());
- unsafe {
- v.as_mut_ptr()
- .copy_from_nonoverlapping(arr.data as *const T, sz);
- v.set_len(sz);
- }
- Ok(v)
+ let n = self.size() / size_of::<T>();
Review comment:
We probably need to constrain T anyways, @mwillsey and I were trying to move the arrays on to the new bindings but it seems like there are so many unsafe gotchas from previous bindings we are trying to work through. In general I think we should probably change NDArray to be like NDArray<T> where T: DataType or something like that. We could probably just slap a `: Sized` on it for now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mwillsey commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
mwillsey commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497733465
##########
File path: rust/tvm/Cargo.toml
##########
@@ -40,6 +40,10 @@ tvm-macros = { version = "*", path = "../tvm-macros/" }
paste = "0.1"
mashup = "0.1"
once_cell = "^1.3.1"
+pyo3 = { version = "0.11.1", optional = true }
Review comment:
I believe that "^" is implicit, no? https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mwillsey commented on pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
mwillsey commented on pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#issuecomment-701591004
@imalsogreg @jroesch Just to break this out into a comment. Many of the remaining issues on this PR that @imalsogreg commented on stem from the fact that `NDArray` (and `NDArrayContainer`) hide the type of the underlying data. This means that typed conversions must be given the type, and we basically have no choice but to trust it.
Should we block this PR on fixing this? I'm leaning toward no.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497180923
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -184,28 +246,19 @@ impl NDArray {
/// let ctx = Context::cpu(0);
/// let mut ndarray = NDArray::empty(&mut shape, ctx, DataType::from_str("int32").unwrap());
/// ndarray.copy_from_buffer(&mut data);
- /// assert_eq!(ndarray.shape(), Some(&mut shape[..]));
+ /// assert_eq!(ndarray.shape(), shape);
/// assert_eq!(ndarray.to_vec::<i32>().unwrap(), data);
/// ```
pub fn to_vec<T>(&self) -> Result<Vec<T>, NDArrayError> {
Review comment:
I'm confused about how this function works. If I call it as `my_array.to_vec<u8>`, I'll get back a vector with more elements than if I call it as `my_array.to_vec::<u32>`, which seems like one or the other would not correspond to a flattened version of `self`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] jroesch merged pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
jroesch merged pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#issuecomment-701718880
@mwillsey 👍 for merge from me. I didn't mean to block anything, mostly I'm trying to get oriented.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mwillsey commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
mwillsey commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497731935
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -374,28 +430,27 @@ mod tests {
#[test]
fn basics() {
- let shape = &mut [1, 2, 3];
+ let shape = &[1, 2, 3];
let ctx = Context::cpu(0);
+ println!("before empty");
Review comment:
I'm fine with println! in tests, since the output is hidden by default, but that's just me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497808143
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -374,28 +430,27 @@ mod tests {
#[test]
fn basics() {
- let shape = &mut [1, 2, 3];
+ let shape = &[1, 2, 3];
let ctx = Context::cpu(0);
+ println!("before empty");
Review comment:
Yep! They don't bug me, just flagged them in case they were leftovers.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mwillsey commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
mwillsey commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497731475
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -184,28 +246,19 @@ impl NDArray {
/// let ctx = Context::cpu(0);
/// let mut ndarray = NDArray::empty(&mut shape, ctx, DataType::from_str("int32").unwrap());
/// ndarray.copy_from_buffer(&mut data);
- /// assert_eq!(ndarray.shape(), Some(&mut shape[..]));
+ /// assert_eq!(ndarray.shape(), shape);
/// assert_eq!(ndarray.to_vec::<i32>().unwrap(), data);
/// ```
pub fn to_vec<T>(&self) -> Result<Vec<T>, NDArrayError> {
- if !self.shape().is_some() {
- return Err(NDArrayError::EmptyArray);
- }
- let earr = NDArray::empty(
- self.shape().ok_or(NDArrayError::MissingShape)?,
- Context::cpu(0),
- self.dtype(),
- );
- let target = self.copy_to_ndarray(earr)?;
- let arr = target.as_dltensor();
- let sz = self.size().ok_or(NDArrayError::MissingShape)?;
- let mut v: Vec<T> = Vec::with_capacity(sz * mem::size_of::<T>());
- unsafe {
- v.as_mut_ptr()
- .copy_from_nonoverlapping(arr.data as *const T, sz);
- v.set_len(sz);
- }
- Ok(v)
+ let n = self.size() / size_of::<T>();
Review comment:
Yeah i think panicking on div-by-zero would be the correct behavior. Follow up on jared, I think a goal should be to move to NDArray<T>, because right now these conversions are all effectively transmutes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497168051
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -184,28 +246,19 @@ impl NDArray {
/// let ctx = Context::cpu(0);
/// let mut ndarray = NDArray::empty(&mut shape, ctx, DataType::from_str("int32").unwrap());
/// ndarray.copy_from_buffer(&mut data);
- /// assert_eq!(ndarray.shape(), Some(&mut shape[..]));
+ /// assert_eq!(ndarray.shape(), shape);
/// assert_eq!(ndarray.to_vec::<i32>().unwrap(), data);
/// ```
pub fn to_vec<T>(&self) -> Result<Vec<T>, NDArrayError> {
- if !self.shape().is_some() {
- return Err(NDArrayError::EmptyArray);
- }
- let earr = NDArray::empty(
- self.shape().ok_or(NDArrayError::MissingShape)?,
- Context::cpu(0),
- self.dtype(),
- );
- let target = self.copy_to_ndarray(earr)?;
- let arr = target.as_dltensor();
- let sz = self.size().ok_or(NDArrayError::MissingShape)?;
- let mut v: Vec<T> = Vec::with_capacity(sz * mem::size_of::<T>());
- unsafe {
- v.as_mut_ptr()
- .copy_from_nonoverlapping(arr.data as *const T, sz);
- v.set_len(sz);
- }
- Ok(v)
+ let n = self.size() / size_of::<T>();
Review comment:
Could a user ever request `T ~ ()`, or some other zero-sized item, giving a divide-by-zero error?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] jroesch commented on pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#issuecomment-699461877
cc @binarybana @imalsogreg @robo-corg @adelbertc @anwang2009
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497193474
##########
File path: rust/tvm/examples/resnet/build.rs
##########
@@ -17,16 +17,20 @@
* under the License.
*/
-use anyhow::{Context, Result};
-use std::{path::Path, process::Command};
+use std::{io::Write, path::Path, process::Command};
-fn main() -> Result<()> {
+fn main() {
let output = Command::new("python3")
.arg(concat!(env!("CARGO_MANIFEST_DIR"), "/src/build_resnet.py"))
.arg(&format!("--build-dir={}", env!("CARGO_MANIFEST_DIR")))
.output()
- .with_context(|| anyhow::anyhow!("failed to run python3"))?;
Review comment:
Is it intentional here that we're reverting the `anyhow` changes from #6401?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497171514
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -374,28 +430,27 @@ mod tests {
#[test]
fn basics() {
- let shape = &mut [1, 2, 3];
+ let shape = &[1, 2, 3];
let ctx = Context::cpu(0);
+ println!("before empty");
Review comment:
Stale helpers, not needed anymore?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497191902
##########
File path: rust/tvm/Cargo.toml
##########
@@ -40,6 +40,10 @@ tvm-macros = { version = "*", path = "../tvm-macros/" }
paste = "0.1"
mashup = "0.1"
once_cell = "^1.3.1"
+pyo3 = { version = "0.11.1", optional = true }
Review comment:
Nit: `^0.11.1`, to pick up new non-breaking changes? Or do we want `0.11.1` specifically?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497162386
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -47,73 +47,146 @@
//! [`copy_from_buffer`]:struct.NDArray.html#method.copy_from_buffer
//! [`copy_to_ctx`]:struct.NDArray.html#method.copy_to_ctx
-use std::convert::TryInto;
use std::ffi::c_void;
+use std::{borrow::Cow, convert::TryInto};
use std::{convert::TryFrom, mem, os::raw::c_int, ptr, slice, str::FromStr};
-use crate::errors::NDArrayError;
-
+use mem::size_of;
+use tvm_macros::Object;
use tvm_sys::ffi::DLTensor;
use tvm_sys::{ffi, ByteArray, Context, DataType};
use ndarray::{Array, ArrayD};
use num_traits::Num;
+use crate::errors::NDArrayError;
+
+use crate::object::{Object, ObjectPtr};
+
/// See the [`module-level documentation`](../ndarray/index.html) for more details.
-///
-/// Wrapper around TVM array handle.
-#[derive(Debug)]
-pub enum NDArray {
- Borrowed { handle: ffi::TVMArrayHandle },
- Owned { handle: *mut c_void },
+#[repr(C)]
+#[derive(Object)]
+#[ref_name = "NDArray"]
+#[type_key = "runtime.NDArray"]
+pub struct NDArrayContainer {
+ base: Object,
+ // Container Base
+ dl_tensor: DLTensor,
+ manager_ctx: *mut c_void,
+ // TOOD: shape?
}
-impl NDArray {
- pub(crate) fn new(handle: ffi::TVMArrayHandle) -> Self {
- NDArray::Borrowed { handle }
+impl NDArrayContainer {
+ pub(crate) fn from_raw(handle: ffi::TVMArrayHandle) -> Option<ObjectPtr<Self>> {
+ let base_offset = memoffset::offset_of!(NDArrayContainer, dl_tensor) as isize;
+ let base_ptr = unsafe { (handle as *mut i8).offset(-base_offset) };
+ let object_ptr = ObjectPtr::from_raw(base_ptr.cast());
+ object_ptr.map(|ptr| {
+ ptr.downcast::<NDArrayContainer>()
+ .expect("we know this is an NDArray container")
+ })
+ }
+
+ pub fn leak<'a>(object_ptr: ObjectPtr<NDArrayContainer>) -> &'a mut NDArrayContainer
+ where
+ NDArrayContainer: 'a,
+ {
+ let base_offset = memoffset::offset_of!(NDArrayContainer, dl_tensor) as isize;
+ unsafe {
+ &mut *std::mem::ManuallyDrop::new(object_ptr)
+ .ptr
+ .as_ptr()
+ .cast::<u8>()
+ .offset(base_offset)
+ .cast::<NDArrayContainer>()
+ }
}
+}
- pub(crate) fn from_ndarray_handle(handle: *mut c_void) -> Self {
- NDArray::Owned { handle }
+fn cow_usize<'a>(slice: &[i64]) -> Cow<'a, [usize]> {
+ if std::mem::size_of::<usize>() == 64 {
+ debug_assert!(slice.iter().all(|&x| x >= 0));
+ let shape: &[usize] = unsafe { std::mem::transmute(slice) };
+ Cow::Borrowed(shape)
+ } else {
+ let shape: Vec<usize> = slice
+ .iter()
+ .map(|&x| usize::try_from(x).unwrap_or_else(|_| panic!("Cannot fit into usize: {}", x)))
+ .collect();
+ Cow::Owned(shape)
}
+}
- pub fn as_dltensor(&self) -> &DLTensor {
- let ptr: *mut DLTensor = match self {
- NDArray::Borrowed { ref handle } => *handle,
- NDArray::Owned { ref handle } => *handle as *mut DLTensor,
- };
+impl NDArray {
+ pub(crate) fn _from_raw(handle: ffi::TVMArrayHandle) -> Self {
+ let ptr = NDArrayContainer::from_raw(handle);
+ NDArray(ptr)
+ }
- unsafe { std::mem::transmute(ptr) }
+ // I think these should be marked as unsafe functions? projecting a reference is bad news.
Review comment:
Just calling this out as a TODO
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mwillsey commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
mwillsey commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497732844
##########
File path: rust/tvm-rt/src/ndarray.rs
##########
@@ -184,28 +246,19 @@ impl NDArray {
/// let ctx = Context::cpu(0);
/// let mut ndarray = NDArray::empty(&mut shape, ctx, DataType::from_str("int32").unwrap());
/// ndarray.copy_from_buffer(&mut data);
- /// assert_eq!(ndarray.shape(), Some(&mut shape[..]));
+ /// assert_eq!(ndarray.shape(), shape);
/// assert_eq!(ndarray.to_vec::<i32>().unwrap(), data);
/// ```
pub fn to_vec<T>(&self) -> Result<Vec<T>, NDArrayError> {
Review comment:
Yeah, see above. The problem is that NDArray isn't NDArray<T>. So we trust the type of T given to us by `to_vec::<T>`. That makes it effectively a transmute of the buffer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] imalsogreg commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
imalsogreg commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497803337
##########
File path: rust/tvm/Cargo.toml
##########
@@ -40,6 +40,10 @@ tvm-macros = { version = "*", path = "../tvm-macros/" }
paste = "0.1"
mashup = "0.1"
once_cell = "^1.3.1"
+pyo3 = { version = "0.11.1", optional = true }
Review comment:
TIL :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] jroesch commented on pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#issuecomment-701717861
I think we should just add a Rust bindings stabilization tracking issue for all these remaining items and set a stabilization goal on the current API, etc at which we will impose more strict coding standards and clean up.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mwillsey commented on a change in pull request #6563: [Rust] Improve NDArray, GraphRt, and Relay bindings
Posted by GitBox <gi...@apache.org>.
mwillsey commented on a change in pull request #6563:
URL: https://github.com/apache/incubator-tvm/pull/6563#discussion_r497735735
##########
File path: rust/tvm/examples/resnet/build.rs
##########
@@ -17,16 +17,20 @@
* under the License.
*/
-use anyhow::{Context, Result};
-use std::{path::Path, process::Command};
+use std::{io::Write, path::Path, process::Command};
-fn main() -> Result<()> {
+fn main() {
let output = Command::new("python3")
.arg(concat!(env!("CARGO_MANIFEST_DIR"), "/src/build_resnet.py"))
.arg(&format!("--build-dir={}", env!("CARGO_MANIFEST_DIR")))
.output()
- .with_context(|| anyhow::anyhow!("failed to run python3"))?;
Review comment:
Good catch, I'll put it back.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org