-
Notifications
You must be signed in to change notification settings - Fork 154
Enabling monitoring/probing of Vars and OutPorts of processes with Monitor Process #80
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
Conversation
…litate for accessing extra proc param in proc model
� Conflicts: � src/lava/magma/compiler/builder.py � src/lava/magma/compiler/compiler.py � src/lava/magma/core/model/interfaces.py � src/lava/magma/core/model/py/model.py � src/lava/magma/core/model/py/ports.py � src/lava/magma/core/process/ports/ports.py � src/lava/magma/runtime/runtime_service.py � tests/lava/magma/compiler/test_compiler.py � tests/lava/magma/core/model/test_py_model.py � tests/lava/magma/core/process/test_ports.py � tests/lava/magma/runtime/test_ref_var_ports.py
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 commented on a few things to change (naming, comments, simpler code).
Please also fix the linting errors (see checks).
Overall we probably need to refactor this soon to enable probing of multiple Vars, but as a first version this should be good.
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 only did a superficial review in terms of coding style.
Please try to be more careful with your spelling, I found a lot of typos in docstrings.
@elvinhajizada Please make sure to push your resolving changes to the PR before you mark conversations as resolved. |
@mathisrichter I was making changes while resolving conversations to not lose track of what I have resolved |
@mathisrichter and @PhilippPlank your requested changes are addressed. Please have a look and approve if it is all right |
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.
Looks good
* - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * First test and empty src files for Monitor proc * Tests for preliminary Monitor proc/-model; adding prom_params to facilitate for accessing extra proc param in proc model * Preliminary Monitor proc/-model for one proc, one var/output monitoring * Monitor proc/-model for monitoring one Var or OutPort of one proc * Adapting Monitor tests to changes in LIF proc * Addressing requested changes in PR#80 * Addressed change requests from PR #80 * Probing multi-dim vars, probe data convention changed, deprecation warnings for float fixed * Fix for flake8 Co-authored-by: PhilippPlank <philipp.plank@intel.com> Co-authored-by: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com>
* - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * First test and empty src files for Monitor proc * Tests for preliminary Monitor proc/-model; adding prom_params to facilitate for accessing extra proc param in proc model * Preliminary Monitor proc/-model for one proc, one var/output monitoring * Monitor proc/-model for monitoring one Var or OutPort of one proc * Adapting Monitor tests to changes in LIF proc * Addressing requested changes in PR#80 * Addressed change requests from PR lava-nc#80 Co-authored-by: PhilippPlank <philipp.plank@intel.com>
* - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * - Initial enablement of RefPort and VarPorts * First test and empty src files for Monitor proc * Tests for preliminary Monitor proc/-model; adding prom_params to facilitate for accessing extra proc param in proc model * Preliminary Monitor proc/-model for one proc, one var/output monitoring * Monitor proc/-model for monitoring one Var or OutPort of one proc * Adapting Monitor tests to changes in LIF proc * Addressing requested changes in PR#80 * Addressed change requests from PR lava-nc#80 * Probing multi-dim vars, probe data convention changed, deprecation warnings for float fixed * Fix for flake8 Co-authored-by: PhilippPlank <philipp.plank@intel.com> Co-authored-by: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com>
These changes enables monitoring
Vars
andOutPorts
of given process. This is done through Monitor process (implemented here) which dynamically creates newRefPorts
andInPorts
to monitorVars
andOutPorts
, respectively. Each probing also creates a data-stroringVar
. Once run is finished, collected data is structured intodata
attribute of Monitor process. Stored data from specificVars
/OutPorts
of a given Process can be accessed by their names indata
dict.Limitations:
Var
orOutport
of single process.Note: The infrastructure for multi-variable monitoring is mostly implemented and is part of this pull request, however one-to-one correspondence requirement between Process and ProcessModel in terms of LavaPyTypes and Ports, blocks usage of this infrastructure for now.