ml4ai / automates

AutoMATES: Automated Model Assembly from Text, Equations, and Software
https://ml4ai.github.io/automates
Other
25 stars 9 forks source link

Python Global Scope Var IDs Fix #307

Closed titomeister closed 2 years ago

titomeister commented 2 years ago

Fixing an issue that occurs when a global variable at the global scope in a program is used in another Python expression. The issue can be seen with the following code x = 10 y = x + 1

Before the fix, the ID of x in the first line wouldn't match the ID of x in the second line. The variable ID labeling algorithm wasn't properly labeling variables at the global scope if they were used in other lines of Python code.

In addition, this issue would also appear for while loops and if statements at the global level. The fix has also resolved the issues in these cases.

The fix was to make sure that after each global level assignment the IDs get properly updated (line 1472 in py_ast_to_cast.py) as opposed to only doing it after we process all global level assignments.

In addition: another bug was fixed involving variable IDs with while loops. In the following code

x = 10 while x > 0: y = 2 z = y

The variable ID for y on the third line is different than the y in line 4. This is incorrect as y can exist after a while loop even if it isn't declared before (strange Python semantics..) The fix for this is lines 1536 - 1545 in py_ast_to_cast.py.

The way the ID labeling works is that we check a variable's ID by seeing if it exists in the current scope (in this case it's if it exists in the while loop's body or test) and if it doesn't exist then we check the previous scope. If it doesn't exist in either scope then it's created. A special case of this occurs when the previous scope is the global level scope.

Generally, the previous scope is either the scope of the function we're in or the scope of a previous function (in the case of nesting functions). In the scenario above the previous scope is the global scope. At the global level we label the variable_name -> ID maps a little differently. In particular the variable names have their filenames associated with them to help signify they're global to the module they're in. Local variables don't follow this convention. Thus, the fix makes one additional check to see if the full filename + variable_name (called the unique_name in the code) exists in the previous scope, and copies the ID of this unique_name to the current scope ID map.

One final thing: there is currently a bug involving conditionals that introduce variables that haven't been declared before the conditional. This is mentioned in issue #306 .

codecov[bot] commented 2 years ago

Codecov Report

Merging #307 (3d1e21c) into master (389a623) will decrease coverage by 0.01%. The diff coverage is 0.00%.

:exclamation: Current head 3d1e21c differs from pull request most recent head 1b87684. Consider uploading reports for the commit 1b87684 to get more accurate results

@@            Coverage Diff             @@
##           master     #307      +/-   ##
==========================================
- Coverage   66.89%   66.88%   -0.02%     
==========================================
  Files         106      106              
  Lines       21725    21729       +4     
==========================================
  Hits        14534    14534              
- Misses       7191     7195       +4     
Impacted Files Coverage Δ
...ates/program_analysis/PyAST2CAST/py_ast_to_cast.py 12.41% <0.00%> (-0.07%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 389a623...1b87684. Read the comment docs.

cl4yton commented 2 years ago

Thanks @titomeister ! Go ahead an squash-and-merge!