Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

Overload more operators in R for ndarray #226

Merged
tqchen merged 4 commits into from
Oct 7, 2015
Merged

Overload more operators in R for ndarray #226

tqchen merged 4 commits into from
Oct 7, 2015

Conversation

@hetong007
Copy link
Contributor

@hetong007 hetong007 commented Oct 7, 2015

No description provided.

tqchen
tqchen reviewed Oct 7, 2015
View reviewed changes
R-package/R/ndarray.R Outdated
Copy link
Member

@tqchen tqchen Oct 7, 2015

Choose a reason for hiding this comment

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

be a bit consistent with style, space after comma

@tqchen
Copy link
Member

tqchen commented Oct 7, 2015

cc @thirdwing

tqchen added a commit that referenced this pull request Oct 7, 2015
@tqchen
Merge pull request #226 from hetong007/master
6537294 
Overload more operators in R for ndarray
@tqchen tqchen merged commit 6537294 into apache:master Oct 7, 2015
@thirdwing
Copy link
Contributor

thirdwing commented Oct 7, 2015

Both method is OK.

As I remembr, +.Rcpp_MXDArray is for S3 and S4, setMethods is only for S4.

@tqchen
Copy link
Member

tqchen commented Oct 7, 2015

Rcpp_MXNDArray is S4 object

@thirdwing
Copy link
Contributor

thirdwing commented Oct 8, 2015

That is why I said both are OK. We can follow this method.

PS: You won't see warnings below using +.Rcpp_MXDArray.

** R
** demo
** inst
** preparing package for lazy loading
in method for ‘+’ with signature ‘e1="Rcpp_MXNDArray",e2="numeric"’: no definition for class “Rcpp_MXNDArray”
in method for ‘+’ with signature ‘e1="Rcpp_MXNDArray",e2="Rcpp_MXNDArray"’: no definition for class “Rcpp_MXNDArray”
in method for ‘+’ with signature ‘e1="numeric",e2="Rcpp_MXNDArray"’: no definition for class “Rcpp_MXNDArray”
in method for ‘-’ with signature ‘e1="Rcpp_MXNDArray",e2="numeric"’: no definition for class “Rcpp_MXNDArray”
in method for ‘-’ with signature ‘e1="Rcpp_MXNDArray",e2="Rcpp_MXNDArray"’: no definition for class “Rcpp_MXNDArray”
in method for ‘-’ with signature ‘e1="numeric",e2="Rcpp_MXNDArray"’: no definition for class “Rcpp_MXNDArray”
in method for ‘*’ with signature ‘e1="Rcpp_MXNDArray",e2="numeric"’: no definition for class “Rcpp_MXNDArray”
in method for ‘*’ with signature ‘e1="Rcpp_MXNDArray",e2="Rcpp_MXNDArray"’: no definition for class “Rcpp_MXNDArray”
in method for ‘*’ with signature ‘e1="numeric",e2="Rcpp_MXNDArray"’: no definition for class “Rcpp_MXNDArray”
in method for ‘/’ with signature ‘e1="Rcpp_MXNDArray",e2="numeric"’: no definition for class “Rcpp_MXNDArray”
in method for ‘/’ with signature ‘e1="Rcpp_MXNDArray",e2="Rcpp_MXNDArray"’: no definition for class “Rcpp_MXNDArray”
in method for ‘/’ with signature ‘e1="numeric",e2="Rcpp_MXNDArray"’: no definition for class “Rcpp_MXNDArray”

@tqchen
Copy link
Member

tqchen commented Oct 8, 2015

I thought the warning will not happen if the setmethods are in https://github.com/dmlc/mxnet/blob/master/R-package/R/ndarray.R#L4 ? It did not happen on my side

@tqchen
Copy link
Member

tqchen commented Oct 8, 2015

Because init.nd.array is called in onLoad after the library loading

@thirdwing
Copy link
Contributor

thirdwing commented Oct 8, 2015

Defining a init.ndarray.methods and calling it during startup is not a "common" strategy, but it is OK.

I removed it and saw warnings.

@tqchen
Copy link
Member

tqchen commented Oct 8, 2015

I see, this was really due to our tricky requirement of handling the loading orders.. I thought we can go for the S4 way as it is consider more advanced?

We can think of a more formal way later. But I am afraid we do need a init function solution, due to library loading order problem

stefanhenneking pushed a commit to stefanhenneking/mxnet that referenced this pull request Jun 30, 2017
@cjolivier01 @tqchen
Fix link error for newer gcc versions, Minor CMake adjustments (apach…
22e23f1 
…e#226)

* Fix warnings, build adjustments in CMake

* fix cuda variable-declaration order problems

* Remove gpu flavors below 30, no longer compatible

* fix cuda variable-declaration order problems

* Add nvcc --run cuda library path flag for when detecting gpu flavor

* Allow CUDA_LIBRARY_PATH to not be defined

* Minor visibility adjustments

* Remove more warnings

* use static_cast instead of ()

* Fix warnings

* Fix link error, undefined kSize

* Size() for kPlain packet

* fix: Ignored test directory

* Fix lint, re-apply kSize fix
iblislin pushed a commit to iblislin/incubator-mxnet that referenced this pull request Mar 18, 2018
@vchuravy
Merge pull request apache#226 from facundoq/master
867b1c5 
Check if key stride exists in the node info when producing graphviz output
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

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

@hetong007 @tqchen @thirdwing