Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process positional arguments #134

Open
wants to merge 61 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
df69fd5
parsing positional arguments for tf.function decorator
tatianacv Dec 12, 2022
3b881a6
fixing comments
tatianacv Dec 12, 2022
2899de3
adding tests
tatianacv Dec 12, 2022
731b664
fix trailing whitespace
tatianacv Dec 12, 2022
a4cf2cd
fix
tatianacv Dec 12, 2022
d6b16ed
not counting the default values as existing
tatianacv Dec 13, 2022
aeca489
fix, might need to tidy up
tatianacv Dec 13, 2022
61f1b1b
fix
tatianacv Dec 13, 2022
e8b508c
fetching from upstream
tatianacv Dec 14, 2022
109e0e4
Progress
tatianacv Dec 14, 2022
b7d9d5c
fix trailing whitespace
tatianacv Dec 14, 2022
6dbbadd
Throwing exceptions
tatianacv Dec 15, 2022
e5c5b80
adding tests for another tf version
tatianacv Dec 16, 2022
1046cff
changing test's tf.function version and some formatting
tatianacv Dec 17, 2022
8c489aa
fixing python code
tatianacv Dec 17, 2022
130f807
fixing to a non-default value
tatianacv Dec 17, 2022
55cadea
update
tatianacv Dec 17, 2022
c83eccf
adding requirement
tatianacv Dec 17, 2022
4361e2e
update
tatianacv Dec 17, 2022
53c44f1
fix checkstyle
tatianacv Dec 17, 2022
23ae949
update
tatianacv Jan 3, 2023
94a005f
fixing comments
tatianacv Jan 9, 2023
528a3a7
Merge branch 'ponder-lab:main' into issue_108
tatianacv Jan 9, 2023
a3cfae6
progress on comments
tatianacv Jan 9, 2023
92cf918
Merge branch 'issue_108' of https://github.com/tatianacv/Hybridize-Fu…
tatianacv Jan 9, 2023
b145a53
progress
tatianacv Jan 9, 2023
72d53c6
removing code that has to do with keywords, not positionals
tatianacv Jan 10, 2023
d05963b
Adding newline
tatianacv Jan 10, 2023
fffd0d0
removing code that accounts for default values, adding documentation,…
tatianacv Jan 13, 2023
49065c9
removing a character from a comment
tatianacv Jan 13, 2023
f53fe07
Merge branch 'main' into issue_108
khatchad Jan 18, 2023
e52d371
changing the versions to constants
tatianacv Jan 19, 2023
ca0c4d2
add comments regarding the declaring defintions
tatianacv Jan 19, 2023
914c417
Merge branch 'issue_108' of https://github.com/tatianacv/Hybridize-Fu…
tatianacv Jan 19, 2023
ebd3880
changing variable name
tatianacv Jan 19, 2023
aa4f6cd
Merge branch 'main' into issue_108
khatchad Jan 23, 2023
19a8a72
fixing issue #, changing order of assert clause
tatianacv Jan 23, 2023
f92872c
Merge branch 'issue_108' of https://github.com/tatianacv/Hybridize-Fu…
tatianacv Jan 23, 2023
1eea97a
Merge branch 'main' into issue_108
khatchad Jan 24, 2023
f7cd804
Update
tatianacv Jan 26, 2023
a107678
Merge branch 'main' into issue_108
tatianacv Jan 31, 2023
a835e8b
Pass build
tatianacv Feb 24, 2023
594ea9d
Adding more comments
tatianacv Feb 24, 2023
dbf19ad
Merge branch 'main' into issue_108
tatianacv Mar 4, 2023
c182a37
Updates on requested changes
tatianacv Mar 6, 2023
d3239b4
update
tatianacv Mar 10, 2023
9a60cbf
update
tatianacv Mar 10, 2023
1cb7502
Renaming for clarity
tatianacv Mar 10, 2023
f710611
update
tatianacv Mar 10, 2023
dacd17a
Merge branch 'main' into issue_108
khatchad Mar 16, 2023
5673690
update
tatianacv Mar 17, 2023
474acdf
Merge branch 'issue_108' of https://github.com/tatianacv/Hybridize-Fu…
tatianacv Mar 17, 2023
ccce776
Merge branch 'main' into issue_108
khatchad Mar 20, 2023
4e16587
Merge branch 'main' into issue_108
tatianacv Mar 21, 2023
5d72476
Changing comments, removing keyword args else
tatianacv Mar 22, 2023
b4358fd
Merge branch 'issue_108' of https://github.com/tatianacv/Hybridize-Fu…
tatianacv Mar 22, 2023
54ba275
Checking that the size of the parse tffunction args is less than defi…
tatianacv Mar 22, 2023
afd01ff
Adding more info
tatianacv Mar 23, 2023
1ca2997
Merge branch 'main' into issue_108
tatianacv Mar 23, 2023
afdbbb0
Merge branch 'main' into issue_108
khatchad Mar 28, 2023
4161ad7
Merge branch 'main' into issue_108
tatianacv Mar 31, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,21 @@
import static org.eclipse.core.runtime.Platform.getLog;

import java.io.File;
import java.util.ArrayList;
import java.util.Objects;
import java.util.Set;

import org.eclipse.core.runtime.ILog;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.IDocument;
import org.python.pydev.ast.codecompletion.revisited.visitors.Definition;
import org.python.pydev.core.IPythonNature;
import org.python.pydev.core.docutils.PySelection;
import org.python.pydev.parser.jython.ast.Attribute;
import org.python.pydev.parser.jython.ast.Call;
import org.python.pydev.parser.jython.ast.FunctionDef;
import org.python.pydev.parser.jython.ast.Name;
import org.python.pydev.parser.jython.ast.NameTok;
import org.python.pydev.parser.jython.ast.argumentsType;
import org.python.pydev.parser.jython.ast.decoratorsType;
Expand Down Expand Up @@ -105,6 +109,9 @@ public HybridizationParameters(IProgressMonitor monitor) throws BadLocationExcep
// Will contain the last tf.function decorator
decoratorsType tfFunctionDecorator = null;

// Declaring definitions of the decorator
tatianacv marked this conversation as resolved.
Show resolved Hide resolved
Set<Definition> declaringDefinitions = null;

// Iterate through the decorators of the function
for (decoratorsType decorator : decoratorArray) {
IDocument document = Function.this.getContainingDocument();
Expand All @@ -113,54 +120,213 @@ public HybridizationParameters(IProgressMonitor monitor) throws BadLocationExcep
// Save the hybrid decorator
try {
if (Function.isHybrid(decorator, Function.this.containingModuleName, Function.this.containingFile, selection,
Function.this.nature, monitor)) // TODO: Cache this from a previous call (#118).
Function.this.nature, monitor)) { // TODO: Cache this from a previous call (#118).
tfFunctionDecorator = decorator;
declaringDefinitions = Util.getDeclaringDefinition(selection, Function.this.containingModuleName,
tatianacv marked this conversation as resolved.
Show resolved Hide resolved
Function.this.containingFile, Function.this.nature, monitor);
}
} catch (AmbiguousDeclaringModuleException e) {
throw new IllegalStateException("Can't determine whether decorator: " + decorator + " is hybrid.", e);
}
} // We expect to have the last tf.function decorator in tfFunctionDecorator

// Declaring definition of the decorator
Definition declaringDefinition = null;
tatianacv marked this conversation as resolved.
Show resolved Hide resolved

// Getting the definition, there should only be one in the set.
tatianacv marked this conversation as resolved.
Show resolved Hide resolved
if (declaringDefinitions != null) {
declaringDefinition = declaringDefinitions.iterator().next();
}

// Python source arguments from the declaring definition
exprType[] declaringArguments = null;

// Getting the arguments from TensorFlow source
if (declaringDefinition != null) {
if (declaringDefinition.ast instanceof FunctionDef) {
FunctionDef declaringFunctionDefinition = (FunctionDef) declaringDefinition.ast;
argumentsType declaringArgumentTypes = declaringFunctionDefinition.args;
declaringArguments = declaringArgumentTypes.args;
}
}

// Python source arguments from the declaring definition
ArrayList<String> argumentIdDeclaringDefintion = new ArrayList<>();

// Getting the arguments from the definition
if (declaringArguments != null) {
for (exprType declaredArgument : declaringArguments) {
if (declaredArgument instanceof Name) {
Name argumentName = (Name) declaredArgument;
argumentIdDeclaringDefintion.add(argumentName.id);
}
}
}
tatianacv marked this conversation as resolved.
Show resolved Hide resolved

if (tfFunctionDecorator != null)
// tfFunctionDecorator must be an instance of Call, because that's the only way we have parameters.
if (tfFunctionDecorator.func instanceof Call) {
Call callFunction = (Call) tfFunctionDecorator.func;
// We only care about the actual keywords for now.
// TODO: Parse positional arguments (#108).

// Processing positional arguments for tf.function a
exprType[] arguments = callFunction.args;
for (int i = 0; i < arguments.length; i++) {
String argumentDeclaringDefinition = argumentIdDeclaringDefintion.get(i);
Copy link
Member

Choose a reason for hiding this comment

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

Why not use a for each loop here? I don't see i being used elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

I use it in line 182, the i represents the position in which the argument is, then we tie it in with the tf.function definition arguments.


// Matching the arguments from the definition and the arguments from the code being analyzed.
if (argumentDeclaringDefinition.equals(FUNC)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "None")
// Found parameter func
this.funcParamExists = true;
} else {
// Found parameter func
this.funcParamExists = true;
}
} else if (argumentDeclaringDefinition.equals(INPUT_SIGNATURE)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "None")
// Found parameter input_signature
this.inputSignatureParamExists = true;
} else {
// Found parameter input_signature
this.inputSignatureParamExists = true;
}
} else if (argumentDeclaringDefinition.equals(AUTOGRAPH)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "True")
// Found parameter autograph
this.autoGraphParamExists = true;
} else {
// Found parameter autograph
this.autoGraphParamExists = true;
}
// The latest version of the API we are using allows
// parameter names jit_compile and
// deprecated name experimental_compile
} else if (argumentDeclaringDefinition.equals(JIT_COMPILE)
|| argumentDeclaringDefinition.equals(EXPERIMENTAL_COMPILE)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "None")
// Found parameter jit_compile/experimental_compile
this.jitCompileParamExists = true;
} else {
// Found parameter jit_compile/experimental_compile
this.jitCompileParamExists = true;
}
// The latest version of the API we are using allows
// parameter names reduce_retracing
// and deprecated name experimental_relax_shapes
} else if (argumentDeclaringDefinition.equals(REDUCE_RETRACING)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "False")
// Found parameter reduce_retracing
this.reduceRetracingParamExists = true;
} else {
// Found parameter reduce_retracing
this.reduceRetracingParamExists = true;
}
} else if (argumentDeclaringDefinition.equals(EXPERIMENTAL_RELAX_SHAPES)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "None")
// Found parameter experimental_relax_shapes
this.reduceRetracingParamExists = true;
} else {
// Found parameter experimental_relax_shapes
this.reduceRetracingParamExists = true;
}
} else if (argumentDeclaringDefinition.equals(EXPERIMENTAL_IMPLEMENTS)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "None")
// Found parameter experimental_implements
this.experimentalImplementsParamExists = true;
} else {
// Found parameter experimental_implements
this.experimentalImplementsParamExists = true;
}
} else if (argumentDeclaringDefinition.equals(EXPERIMENTAL_AUTOGRAPH_OPTIONS)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "None")
// Found parameter experimental_autograph_options
this.experimentalAutographOptionsParamExists = true;
} else {
// Found parameter experimental_autograph_options
this.experimentalAutographOptionsParamExists = true;
}
} else if (argumentDeclaringDefinition.equals(EXPERIMENTAL_FOLLOW_TYPE_HINTS)) {
// Not considering the default values
if (arguments[i] instanceof Name) {
Name nameArgument = (Name) arguments[i];
if (nameArgument.id != "None")
// Found parameter experimental_follow_type_hints
this.experimentaFollowTypeHintsParamExists = true;
} else {
// Found parameter experimental_follow_type_hints
this.experimentaFollowTypeHintsParamExists = true;
}
}
}

// Processing keywords arguments
// If we have keyword parameter, afterwards, we cannot have positional parameters because it would result in invalid
// Python code.
keywordType[] keywords = callFunction.keywords;
for (keywordType keyword : keywords) {
if (keyword.arg instanceof NameTok) {
NameTok name = (NameTok) keyword.arg;
if (name.id.equals(FUNC))
if (name.id.equals(FUNC) && argumentIdDeclaringDefintion.contains(name.id))
// Found parameter func
this.funcParamExists = true;
else if (name.id.equals(INPUT_SIGNATURE))
else if (name.id.equals(INPUT_SIGNATURE) && argumentIdDeclaringDefintion.contains(name.id))
// Found parameter input_signature
this.inputSignatureParamExists = true;
else if (name.id.equals(AUTOGRAPH))
else if (name.id.equals(AUTOGRAPH) && argumentIdDeclaringDefintion.contains(name.id))
// Found parameter autograph
this.autoGraphParamExists = true;
// The version of the API we are using allows
// The latest version of the API we are using allows
tatianacv marked this conversation as resolved.
Show resolved Hide resolved
// parameter names jit_compile and
// deprecated name experimental_compile
else if (name.id.equals(JIT_COMPILE) || name.id.equals(EXPERIMENTAL_COMPILE))
else if ((name.id.equals(JIT_COMPILE) || name.id.equals(EXPERIMENTAL_COMPILE))
&& argumentIdDeclaringDefintion.contains(name.id))
// Found parameter jit_compile/experimental_compile
this.jitCompileParamExists = true;
// The version of the API we are using allows
// The latest version of the API we are using allows
// parameter names reduce_retracing
// and deprecated name experimental_relax_shapes
else if (name.id.equals(REDUCE_RETRACING) || name.id.equals(EXPERIMENTAL_RELAX_SHAPES))
else if ((name.id.equals(REDUCE_RETRACING) || name.id.equals(EXPERIMENTAL_RELAX_SHAPES))
&& argumentIdDeclaringDefintion.contains(name.id))
// Found parameter reduce_retracing
// or experimental_relax_shapes
this.reduceRetracingParamExists = true;
else if (name.id.equals(EXPERIMENTAL_IMPLEMENTS))
else if (name.id.equals(EXPERIMENTAL_IMPLEMENTS) && argumentIdDeclaringDefintion.contains(name.id))
// Found parameter experimental_implements
this.experimentalImplementsParamExists = true;
else if (name.id.equals(EXPERIMENTAL_AUTOGRAPH_OPTIONS))
else if (name.id.equals(EXPERIMENTAL_AUTOGRAPH_OPTIONS) && argumentIdDeclaringDefintion.contains(name.id))
// Found parameter experimental_autograph_options
this.experimentalAutographOptionsParamExists = true;
else if (name.id.equals(EXPERIMENTAL_FOLLOW_TYPE_HINTS))
else if (name.id.equals(EXPERIMENTAL_FOLLOW_TYPE_HINTS) && argumentIdDeclaringDefintion.contains(name.id))
// Found parameter experimental_follow_type_hints
this.experimentaFollowTypeHintsParamExists = true;
else {
throw new IllegalArgumentException(String.format("The tf.function argument " + name.id)
+ " is not supported in this tool. This tool supports up to v2.9");
}
}
}
} // else, tf.function is used without parameters.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,21 @@
public class Util {

private static final ILog LOG = getLog(Util.class);

tatianacv marked this conversation as resolved.
Show resolved Hide resolved
/**
* Get the name of the module defining the entity described in the given {@link PySelection}.
* Get the name of the definition of the entity described in the given {@link PySelection}.
*
* @param selection The {@link PySelection} in question.
* @param containingModName The name of the module containing the {@link PySelection}.
* @param containingFile The {@link File} containing the module.
* @param nature The {@link IPythonNature} to use.
* @param monitor The IProgressMonitor to use.
* @return The name of the module defining the given {@link PySelection}.
* @return The definition of {@link PySelection}.
* @throws AmbiguousDeclaringModuleException On ambiguous definitions found.
* @throws BadLocationException On a parsing error.
*/
public static String getDeclaringModuleName(PySelection selection, String containingModName, File containingFile, IPythonNature nature,
IProgressMonitor monitor) throws BadLocationException, AmbiguousDeclaringModuleException {
monitor.beginTask("Getting declaring module name.", 1);

LOG.info(String.format("Getting declaring module name for selection: %s in line: %s, module: %s, file: %s, and project: %s.",
selection.getSelectedText(), selection.getLineWithoutCommentsOrLiterals().strip(), containingModName, containingFile,
nature.getProject()));
public static Set<Definition> getDeclaringDefinition(PySelection selection, String containingModName, File containingFile,
IPythonNature nature, IProgressMonitor monitor) throws BadLocationException, AmbiguousDeclaringModuleException {

RefactoringRequest request = new RefactoringRequest(containingFile, selection, nature);

Expand All @@ -78,13 +73,49 @@ public static String getDeclaringModuleName(PySelection selection, String contai
containingFile.getName(), nature.getProject()));

// Collect the potential declaring module names.
Set<String> potentialDeclaringModuleNames = new HashSet<>();
Set<Definition> potentialDeclaringDefinitions = new HashSet<>();

// for each match.
for (ItemPointer itemPointer : pointers) {
Definition definition = itemPointer.definition;
LOG.info("Found definition: " + definition + ".");

// add it to the set of found module names.
potentialDeclaringDefinitions.add(definition);
}

return potentialDeclaringDefinitions;

tatianacv marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Get the name of the module defining the entity described in the given {@link PySelection}.
*
* @param selection The {@link PySelection} in question.
* @param containingModName The name of the module containing the {@link PySelection}.
* @param containingFile The {@link File} containing the module.
* @param nature The {@link IPythonNature} to use.
* @param monitor The IProgressMonitor to use.
* @return The name of the module defining the given {@link PySelection}.
* @throws AmbiguousDeclaringModuleException On ambiguous definitions found.
* @throws BadLocationException On a parsing error.
*/
public static String getDeclaringModuleName(PySelection selection, String containingModName, File containingFile, IPythonNature nature,
IProgressMonitor monitor) throws BadLocationException, AmbiguousDeclaringModuleException {
monitor.beginTask("Getting declaring module name.", 1);

LOG.info(String.format("Getting declaring module name for selection: %s in line: %s, module: %s, file: %s, and project: %s.",
selection.getSelectedText(), selection.getLineWithoutCommentsOrLiterals().strip(), containingModName, containingFile,
nature.getProject()));

Set<Definition> definitions = getDeclaringDefinition(selection, containingModName, containingFile, nature, monitor);

// Collect the potential declaring module names.
Set<String> potentialDeclaringModuleNames = new HashSet<>();

// for each definition.
for (Definition definition : definitions) {

IModule module = definition.module;
LOG.info(String.format("Found module: %s.", module));

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import tensorflow as tf

@tf.function(reduce_retracing=True)
def test(x):
return x

if __name__ == '__main__':
number = tf.constant([1.0, 1.0])
test(number)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tensorflow==2.8.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import tensorflow as tf

@tf.function(None)
def test():
pass

if __name__ == '__main__':
test()
tatianacv marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tensorflow==2.9.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import tensorflow as tf

@tf.function(None, (tf.TensorSpec(shape=[None], dtype=tf.float32),), False, True, True, "google.matmul_low_rank_matrix", tf.autograph.experimental.Feature.EQUALITY_OPERATORS, True, None, False)
def test(x: tf.Tensor):
return x

if __name__ == '__main__':
number = tf.constant([1.0, 1.0])
print(test(number))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tensorflow==2.9.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import tensorflow as tf

@tf.function(None,None,True,None,False,None,None,None, None,None)
tatianacv marked this conversation as resolved.
Show resolved Hide resolved
def test():
pass

if __name__ == '__main__':
test()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tensorflow==2.9.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import tensorflow as tf

@tf.function(None, (tf.TensorSpec(shape=[None], dtype=tf.float32),), False, True, "google.matmul_low_rank_matrix")
def test(x):
return x

if __name__ == '__main__':
number = tf.constant([1.0, 1.0])
test(number)
Loading