This repository was archived by the owner on Nov 17, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
[MXNET-913] Java API --- Scala NDArray Improvement #12536
Closed
Closed
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
507fb63
add initial commit for java NDArray
lanking520 61ed5f6
add operator named functions
lanking520 824b1cd
add support for using new Type-safe api
lanking520 8aabde0
fix Scala Macros failure
lanking520 d74f881
add Java compatible initialize methods and optimized toString
lanking520 ca27685
remove the require field and rename the toString method
lanking520 4cb03d6
revert changes on Macros and will apply in different PR
lanking520 494b7b5
adding new NDArray test
lanking520 bf8792d
remove unrelated method to Java
lanking520 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
69 changes: 69 additions & 0 deletions
69
scala-package/core/src/main/scala/org/apache/mxnet/api/java/ArgBuilder.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.mxnet.api.java | ||
|
|
||
| import scala.collection.mutable | ||
| import scala.collection.mutable.ListBuffer | ||
| import collection.JavaConverters._ | ||
|
|
||
| /** | ||
| * This arg Builder is intent to solve Java to Scala conversion | ||
| * to take the input such as (arg: Any*) | ||
| */ | ||
| class ArgBuilder { | ||
| private var data = ListBuffer[Any]() | ||
| private var map = mutable.Map[String, Any]() | ||
|
|
||
| def addArg(anyRef: AnyRef): ArgBuilder = { | ||
| require(map.isEmpty, | ||
| "Map is not empty, please do either key-value or positional-arg but not both") | ||
| this.data += anyRef.asInstanceOf[Any] | ||
| this | ||
| } | ||
|
|
||
| def addArg(key : String, value : AnyRef) : ArgBuilder = { | ||
| require(data.isEmpty, | ||
| "Data is not empty, please do either key-value or positional-arg but not both") | ||
| this.map(key) = value.asInstanceOf[Any] | ||
| this | ||
| } | ||
|
|
||
| def addBatchArgs(list : java.util.List[AnyRef]) : ArgBuilder = { | ||
| require(map.isEmpty, | ||
| "Map is not empty, please do either key-value or positional-arg but not both") | ||
| for (i <- 0 to list.size()) { | ||
| this.data += list.get(i) | ||
| } | ||
| this | ||
| } | ||
|
|
||
| def addBatchArgs(arr : Array[AnyRef]) : ArgBuilder = { | ||
| require(map.isEmpty, | ||
| "Map is not empty, please do either key-value or positional-arg but not both") | ||
| arr.foreach(ele => this.data += ele) | ||
| this | ||
| } | ||
|
|
||
| def buildMap() : Map[String, Any] = { | ||
| this.map.toMap | ||
| } | ||
|
|
||
| def buildSeq() : Seq[Any] = { | ||
| this.data | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this? why are you going back in time? we want to move away from (arg: Any*) to type-safe APIs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think type-safe is not as important as ease of usage. For type-safe in Java, the major question is how to deal with default args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess Java does not get defaults. @andrewfayres rightly asked how many APIs do we have that have a large number of parameters to matter? may be we just add builder to those if they are only a few and for others they can pass Scala Option.None or change the APIs to accept gauva Optional.
@lanking520 can you find how many have more than 5 parameters?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After running this:
get output
However, we should consider
outas a param in Type-safe API param. Counting this we gotThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not the total number of parameters that matters, it's how many default parameters there are in the method. If a method has 5 parameters and no defaults then the builder doesn't help any. If there are 5 parameters and all are defaults then it helps a lot.
@lanking520 Can we get a count of how many methods have more than 3 default args? If possible what I'd really like is a distribution (x methods have 1 default arg, y have 2, ...) but if this is too difficult I understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So about 10% of NDArray methods have more than 3 default args.
I'm going to give some more thought before I commit to this but my initial reaction is that we include this but try not to promote it's use too much in docs/examples. Leave it there as an ease of use option for the customer with the understanding that when they use this they will be giving up type-safety.