Refactor/execution mode compiled #2550
Conversation
- learn(): add error for use of ExecutionMode.LLVM or ExecutionMode.PyTorch • autodiffcomposition.py - learn(): add warning for use of ExecutionMode.Python • test_learning.py: - add test_execution_mode_pytorch_and_LLVM_errors • test_autodiffcomposition.py: - add test_execution_mode_python_warning
- add ExecutionMode.COMPILED (~ Python | PyTorch). • compositon.py, autodiffcomposition.py, compositionrunner.py: - modifed to sue ExecutionMode.COMPILED for condionals • test_autodiffcomposition.py: - test_execution_mode_python_error() replaces test_execution_mode_python_warning()
|
This PR causes the following changes to the html docs (ubuntu-latest-3.7-x64): See CI logs for the full diff. |
| @@ -10089,7 +10089,7 @@ def run( | |||
| elif execution_mode & pnlvm.ExecutionMode.PTX: | |||
| results += _comp_ex.cuda_run(inputs, num_trials, num_inputs_sets) | |||
| else: | |||
| assert False, "Unknown execution mode: {}".format(execution_mode) | |||
| assert False, f"Unknown execution mode: {execution_mode}." | |||
There was a problem hiding this comment.
.format is preferred to f-strings at least in compilation related code.
| @@ -10721,7 +10721,7 @@ def execute( | |||
| elif execution_mode & pnlvm.ExecutionMode.PTX: | |||
| _comp_ex.cuda_execute(llvm_inputs) | |||
| else: | |||
| assert False, "Unknown execution mode: {}".format(execution_mode) | |||
| assert False, f"Unknown execution mode: {execution_mode}." | |||
There was a problem hiding this comment.
.format is preferred to f-string
| @@ -10395,7 +10395,7 @@ def learn( | |||
| from psyneulink.library.compositions import AutodiffComposition | |||
| runner = CompositionRunner(self) | |||
|
|
|||
| if (execution_mode in {pnlvm.ExecutionMode.PyTorch, pnlvm.ExecutionMode.LLVM} | |||
| if ((execution_mode is not pnlvm.ExecutionMode.Python) | |||
There was a problem hiding this comment.
I think this will complain about using is instead of ==.
There was a problem hiding this comment.
Comparing enum member not its value.
| context.execution_phase = ContextFlags.PROCESSING | ||
| self.controller.execute(context=context) | ||
|
|
||
| if execution_mode: | ||
| if execution_mode & pnlvm.ExecutionMode.COMPILED: |
There was a problem hiding this comment.
This (and all other checks before execute_node) can use an assertion:
assert execution_mode == pnlvm.ExecutionMode.LLVM or execution_mode & pnl.ExecutionMode._Fallback
Those two cases are the only way to reach the code.
There was a problem hiding this comment.
Applied assertion here, but other checks (at least on composition.py) seem to be part of legitimate branching conditions, not simple else's.
| f"or a standard Composition for Python execution.)") | ||
| raise AutodiffCompositionError(f"{self.name} is an AutodiffComposition so its learn() " | ||
| f"cannot be called with execution_mode = ExecutionMode.Python; " | ||
| f"use ExecutionMode.PyTorch or ExecutionMode.LLVM.") |
There was a problem hiding this comment.
I think the only compiled mode implemented for autodiff is LLVMRun.
|
This pull request introduces 1 alert when merging b5e882f into 26bf706 - view on LGTM.com new alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed 🚀. For more information, please check out our post on the GitHub blog. |
|
This PR causes the following changes to the html docs (ubuntu-latest-3.7-x64): See CI logs for the full diff. |
- test_execution_mode_python_error(): fix bug
|
This PR causes the following changes to the html docs (ubuntu-latest-3.7-x64): See CI logs for the full diff. |
|
This pull request introduces 1 alert when merging cbb1a14 into 26bf706 - view on LGTM.com new alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed 🚀. For more information, please check out our post on the GitHub blog. |
• llvm/init.py
This was added both for clarity at decision points in the code (mainly, composition.py) and to be able to distinguish Python from PyTorch modes.
Now enforces use of ExecutionMode.PyTorch or LLVM for AutdoffiComposition.learn() (to make it clear that execution learning in Autodiff uses one of these, and not Python).
Added documentation in composition.py and autodiffcomposition.py regarding different modes and execution_mode specifications requirements.
• compositon.py, autodiffcomposition.py, compositionrunner.py:
• test_autodiffcomposition.py: