Modifications per recommended changes.

* Document Scope as not offering thread-safety
* Remove synchronized sections within Scope
* Restrict withName() and withSubScope() to only allow names matching
  [A-Za-z0-9.][A-Za-z0-9_.\\-]*
* Consolidate tensorflow and tensorflow_op into one BUILD target
* Fix mistake in opname in Javadoc example
* Change method qualifier in inner test classes to default
* rename baseName -> opPrefix, baseOpName -> opName
This commit is contained in:
KB Sriram 2017-06-08 07:02:11 -07:00 committed by Martin Wicke
parent 3072c2f239
commit 950a38564d
5 changed files with 64 additions and 89 deletions

View File

@ -9,22 +9,15 @@ load("build_defs", "JAVACOPTS")
java_library(
name = "tensorflow",
srcs = [":java_sources"],
srcs = [
":java_op_sources",
":java_sources",
],
data = [":libtensorflow_jni"],
javacopts = JAVACOPTS,
visibility = ["//visibility:public"],
)
# Operator convenience wrappers for the core tensorflow library.
java_library(
name = "tensorflow_op",
srcs = [
":java_op_sources",
],
javacopts = JAVACOPTS,
deps = [":tensorflow"],
)
# NOTE(ashankar): Rule to include the Java API in the Android Inference Library
# .aar. At some point, might make sense for a .aar rule here instead.
filegroup(
@ -164,7 +157,6 @@ java_test(
test_class = "org.tensorflow.op.ScopeTest",
deps = [
":tensorflow",
":tensorflow_op",
":testutil",
"@junit",
],

View File

@ -32,41 +32,34 @@ import java.util.regex.Pattern;
* <p>This class is package private, user code creates {@link Scope} which internally delegates
* calls to an underlying {@code NameScope}.
*
* <p>This class is thread-safe.
* <p>This class is <b>not</b> thread-safe.
*/
final class NameScope {
NameScope withSubScope(String scopeName) {
if (baseName == null) {
checkPattern(OP_NAME_REGEX, scopeName);
} else {
checkPattern(SUBSCOPE_NAME_REGEX, scopeName);
}
checkPattern(NAME_REGEX, scopeName);
if (baseOpName != null) {
// Use the base name instead to derive the subscope.
scopeName = baseOpName;
}
// Override with opName if it exists.
String actualName = (opName != null) ? opName : scopeName;
String newBaseName = fullyQualify(makeUnique(scopeName));
return new NameScope(newBaseName, null, null);
String newPrefix = fullyQualify(makeUnique(actualName));
return new NameScope(newPrefix, null, null);
}
NameScope withName(String name) {
checkPattern(OP_NAME_REGEX, name);
checkPattern(NAME_REGEX, name);
// All context except for the baseOpName is shared with the new scope.
return new NameScope(baseName, name, ids);
// All context except for the opName is shared with the new scope.
return new NameScope(opPrefix, name, ids);
}
String makeOpName(String opName) {
checkPattern(OP_NAME_REGEX, opName);
String makeOpName(String name) {
checkPattern(NAME_REGEX, name);
if (baseOpName != null) {
opName = baseOpName;
}
opName = makeUnique(opName);
return fullyQualify(opName);
// Override with opName if it exists.
String actualName = (opName != null) ? opName : name;
return fullyQualify(makeUnique(actualName));
}
/**
@ -79,9 +72,9 @@ final class NameScope {
this(null, null, null);
}
private NameScope(String baseName, String baseOpName, Map<String, Integer> ids) {
this.baseName = baseName;
this.baseOpName = baseOpName;
private NameScope(String opPrefix, String opName, Map<String, Integer> ids) {
this.opPrefix = opPrefix;
this.opName = opName;
if (ids != null) {
this.ids = ids;
} else {
@ -101,7 +94,6 @@ final class NameScope {
// The second use of makeUnique("a") updates ids to "a" -> 2
// and returns "a_1", and so on.
private String makeUnique(String id) {
synchronized (ids) {
if (!ids.containsKey(id)) {
ids.put(id, 1);
return id;
@ -111,23 +103,22 @@ final class NameScope {
return String.format("%s_%d", id, cur);
}
}
}
private String fullyQualify(String name) {
if (baseName != null) {
return String.format("%s/%s", baseName, name);
if (opPrefix != null) {
return String.format("%s/%s", opPrefix, name);
} else {
return name;
}
}
// If baseName is non-null, it is a prefix applied to all names
// If opPrefix is non-null, it is a prefix applied to all names
// created by this instance.
private final String baseName;
private final String opPrefix;
// If baseOpName is non-null, it is used to derive the unique name
// If opName is non-null, it is used to derive the unique name
// for operators rather than the provided default name.
private final String baseOpName;
private final String opName;
// NameScope generates unique names by appending a numeric suffix if
// needed. This is a map containing names already created by this
@ -153,6 +144,5 @@ final class NameScope {
// LETTER_DIGIT_DOT, followed by zero or more LETTER_DIGIT_DASH_DOT_SLASH_UNDERSCORE. SLASH is not
// permitted in actual user-supplied names to NameScope - it is used as a reserved character to
// separate subcomponents within fully qualified names.
private static final Pattern OP_NAME_REGEX = Pattern.compile("[A-Za-z0-9.][A-Za-z0-9_.\\-]*");
private static final Pattern SUBSCOPE_NAME_REGEX = Pattern.compile("[A-Za-z0-9_.\\-]+");
private static final Pattern NAME_REGEX = Pattern.compile("[A-Za-z0-9.][A-Za-z0-9_.\\-]*");
}

View File

@ -37,7 +37,7 @@ import org.tensorflow.Graph;
* public class Constant {
* public static Constant create(Scope scope, ...) {
* scope.graph().opBuilder(
* "Constant", scope.makeOpName("Constant"))
* "Const", scope.makeOpName("Const"))
* .setAttr(...)
* .build()
* ...
@ -61,17 +61,17 @@ import org.tensorflow.Graph;
* // This op name will be "linear/W"
* Constant.create(linear.withName("W"), ...);
*
* // This op will be "linear/Constant", using the default
* // This op will be "linear/Const", using the default
* // name provided by Constant
* Constant.create(linear, ...);
*
* // This op will be "linear/Constant_1", using the default
* // This op will be "linear/Const_1", using the default
* // name provided by Constant and making it unique within
* // this scope
* Constant.create(linear, ...);
* }</pre>
*
* <p>Scope objects are thread-safe.
* <p>Scope objects are <b>not</b> thread-safe.
*/
public final class Scope {
@ -96,12 +96,7 @@ public final class Scope {
* name will be unique in the returned scope. All other properties are inherited from the current
* scope.
*
* <p>Valid child scope names must match one of the following regular expressions:
*
* <pre>{@code
* [A-Za-z0-9.][A-Za-z0-9_.\\-]* (for scopes at the root)
* [A-Za-z0-9_.\\-]+ (for other scopes)
* }</pre>
* <p>The child scope name must match the regular expression {@code [A-Za-z0-9.][A-Za-z0-9_.\-]*}
*
* @param childScopeName name for the new child scope
* @return a new subscope
@ -117,7 +112,7 @@ public final class Scope {
* <p>Operations created within this scope will have a name of the form {@code
* name/opName[_suffix]}. This lets you name a specific operator more meaningfully.
*
* <p>Valid operator names must match the regular expression {@code [A-Za-z0-9.][A-Za-z0-9_.\\-]*}
* <p>Names must match the regular expression {@code [A-Za-z0-9.][A-Za-z0-9_.\-]*}
*
* @param opName name for an operator in the returned scope
* @return a new Scope that uses opName for operations.
@ -136,7 +131,7 @@ public final class Scope {
* instance. Typical operator building code might look like
*
* <pre>{@code
* scope.graph().opBuilder("Constant", scope.makeOpName("Constant"))...
* scope.graph().opBuilder("Const", scope.makeOpName("Const"))...
* }</pre>
*
* <p><b>Note:</b> if you provide a composite operator building class (i.e, a class that adds a

View File

@ -87,7 +87,7 @@ public class ScopeTest {
Scope root = new Scope(g);
final String[] invalid_names = {
"_", // Names are constrained to start with [A-Za-z0-9.]
"_", "-", "-x", // Names are constrained to start with [A-Za-z0-9.]
null, "", "a$", // Invalid characters
"a/b", // slashes not allowed
};
@ -99,7 +99,7 @@ public class ScopeTest {
} catch (IllegalArgumentException ex) {
// expected
}
// Root scopes follow the same rules as opnames
// Subscopes follow the same rules
try {
root.withSubScope(name);
fail("failed to catch invalid scope name: " + name);
@ -108,8 +108,13 @@ public class ScopeTest {
}
}
// Non-root scopes have a less restrictive constraint.
assertEquals("a/_/hello", root.withSubScope("a").withSubScope("_").makeOpName("hello"));
// Unusual but valid names.
final String[] valid_names = {".", "..", "._-.", "a--."};
for (String name : valid_names) {
root.withName(name);
root.withSubScope(name);
}
}
}
@ -175,7 +180,7 @@ public class ScopeTest {
private static final class Const {
private final Output output;
private static Const create(Scope s, Object v) {
static Const create(Scope s, Object v) {
try (Tensor value = Tensor.create(v)) {
return new Const(
s.graph()
@ -187,11 +192,11 @@ public class ScopeTest {
}
}
private Const(Output o) {
Const(Output o) {
output = o;
}
public Output output() {
Output output() {
return output;
}
}
@ -199,7 +204,7 @@ public class ScopeTest {
private static final class Mean {
private final Output output;
private static Mean create(Scope s, Output input, Output reductionIndices) {
static Mean create(Scope s, Output input, Output reductionIndices) {
return new Mean(
s.graph()
.opBuilder("Mean", s.makeOpName("Mean"))
@ -209,11 +214,11 @@ public class ScopeTest {
.output(0));
}
private Mean(Output o) {
Mean(Output o) {
output = o;
}
public Output output() {
Output output() {
return output;
}
}
@ -221,7 +226,7 @@ public class ScopeTest {
private static final class SquaredDifference {
private final Output output;
private static SquaredDifference create(Scope s, Output x, Output y) {
static SquaredDifference create(Scope s, Output x, Output y) {
return new SquaredDifference(
s.graph()
.opBuilder("SquaredDifference", s.makeOpName("SquaredDifference"))
@ -231,11 +236,11 @@ public class ScopeTest {
.output(0));
}
private SquaredDifference(Output o) {
SquaredDifference(Output o) {
output = o;
}
public Output output() {
Output output() {
return output;
}
}
@ -243,7 +248,7 @@ public class ScopeTest {
private static final class Variance {
private final Output output;
private static Variance create(Scope base, Output x) {
static Variance create(Scope base, Output x) {
Scope s = base.withSubScope("variance");
Output zero = Const.create(s.withName("zero"), new int[] {0}).output();
Output sqdiff =
@ -254,11 +259,11 @@ public class ScopeTest {
return new Variance(Mean.create(s.withName("variance"), sqdiff, zero).output());
}
private Variance(Output o) {
Variance(Output o) {
output = o;
}
public Output output() {
Output output() {
return output;
}
}

View File

@ -20,8 +20,6 @@
# And jars:
# (3) Java API .jar
# (4) Java API sources .jar
# (5) Java Op wrappers .jar
# (6) Java Op wrapper sources .jar
#
# These binary distributions will allow use of TensorFlow in various languages
# without having to compile the TensorFlow framework from sources, which takes
@ -36,8 +34,6 @@
# - lib_package/libtensorflow_jni${SUFFIX}.tar.gz
# - lib_package/libtensorflow.jar
# - lib_package/libtensorflow-src.jar
# - lib_package/libtensorflow_op.jar
# - lib_package/libtensorflow_op-src.jar
# - lib_package/libtensorflow_proto.zip
#
# ASSUMPTIONS:
@ -79,15 +75,12 @@ function build_libtensorflow_tarball() {
//tensorflow/tools/lib_package:libtensorflow_jni.tar.gz \
//tensorflow/java:libtensorflow.jar \
//tensorflow/java:libtensorflow-src.jar \
//tensorflow/java:libtensorflow_op.jar \
//tensorflow/java:libtensorflow_op-src.jar \
//tensorflow/tools/lib_package:libtensorflow_proto.zip
mkdir -p ${DIR}
cp bazel-bin/tensorflow/tools/lib_package/libtensorflow.tar.gz ${DIR}/libtensorflow${TARBALL_SUFFIX}.tar.gz
cp bazel-bin/tensorflow/tools/lib_package/libtensorflow_jni.tar.gz ${DIR}/libtensorflow_jni${TARBALL_SUFFIX}.tar.gz
cp bazel-bin/tensorflow/java/libtensorflow.jar bazel-bin/tensorflow/java/libtensorflow-src.jar ${DIR}
cp bazel-bin/tensorflow/java/libtensorflow_op.jar bazel-bin/tensorflow/java/libtensorflow_op-src.jar ${DIR}
cp bazel-genfiles/tensorflow/tools/lib_package/libtensorflow_proto.zip ${DIR}
chmod -x ${DIR}/*
}