Interactive Console Major Update#1452
Conversation
|
CMAKE-FORMAT TEST - PASSED |
|
CLANG-FORMAT TEST - FAILED (on last commit): |
e6df8ce to
71a2089
Compare
|
CMAKE-FORMAT TEST - PASSED |
|
CLANG-FORMAT TEST - PASSED |
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
Major Changes: - config.h, config.cc Add support for interactive realtime action, sst.rt.interactive (sstsimulator#1266) Add checkpoint-enable flag to enable checkpointing in interactive console. Make sst.interactive.simpledebug the default for interactive console and only use --interactive-console to specify a custom IC File replay option ( --replay ) - from_string.h: Full precision printing of float types. Scientific notation when value is greated than 10**6 or less than 10**-6 - impl/interactive/Makefile.inc Included cmdLineEditor files for build - impl/interactive/cmdLineEditor.h, impl/interactive/cmdLineEditor.cc In-line command editing support - impl/interactive/simpleDebug.h, impl/interactive/simpleDebug.cc Shutdown support (sstsimulator#1311) Watchpoint support Add checkpoint-enabled flag to enable in interactive console. Make sst.rtl.simpledebug the default of interactive console. Only use --interactive-console to specify custom IC. Allow for empty command and other minor whitespace annoyances. Console command logging and replay. Support commented lines for replay scripts. Consolidated commands into a command funtion table. Standardized specifying commands, help text, and command groups Provided auto-completion support for commands and object names Various nullptr checks and fixes to avoid segmentation faults. - interactiveConsole.h Add support for shutdown (sstsimulator#1311) - serialization/objectMap.cc, serialization/objectMap.h Support for trace buffers and watchpoint actions Circular buffer and trigger recording Add ability to check/sample before and after both clock and event handlers. Watchpoint support for adding set var val action. Extended trigger tests functionality: support for logic operators (e.g. v1 < 10 && v2 > 5) and comparisons between two variables Support for trigger comparison between two variables. All set string to handle spaces. watchpoint verbosity control Various bug fixes for command line editing Fixes for nullptr checks and address sanitizer issues causing seg faults. - simulation.cc Interactive console realtime action (sstsimulator#1266) Addition of checkpoint-enable flag to enable checkpoint in IC. Make sst.interactive.simpledebug the default for IC replay option for IC - watchPoint.h, watchPoint.cc Circular buffer, trigger record. Add ability to check/sample before and after both clock and event handlers Add watchpoint action class and add set var val action Add support for trigger comparison between two variables Update watchpoint handling Allow set string to handle spaces Add shutdown as trigger action Add watchpoint index to watchpoint class Add checkpoint-enable flag to enable checkpointing in interactive console. Make sst.interactive.simpledebug the default for interactive console. Only use --interactive-console to specify a custom IC. watchpoint verbosity control
71a2089 to
2944a01
Compare
|
CMAKE-FORMAT TEST - PASSED |
|
CLANG-FORMAT TEST - PASSED |
| else | ||
| s << std::fixed << std::setprecision(std::numeric_limits<double>::max_digits10) << input; | ||
| return s.str().c_str(); | ||
| } |
There was a problem hiding this comment.
I would suggest:
if constexpr ( std::is_floating_point_v<T> ) {
char s[32];
snprintf(s, sizeof(s), "%.*g", std::numeric_limits<T>::max_digits10, input);
return s;
}printf() is much faster and more flexible than C++ <iostream> operators.
The %.*g specifier uses scientific notation for large and small exponents, otherwise not, and its precision, indicated by *, uses the next argument, which std::numeric_limits<T>::max_digits10 returns the number of digits required. This guarantees that a value written out can be read back in by scanf() and the original bits are unchanged.
If a machine is going to be reading/writing the numbers, the %a specifier is exact and prints floating-point in hexadecimal notation.
You probably should add #include <cstdio> at the top of the file. Please run scripts/test-includes.pl to check for needed #includes. It will be added later to CI and maybe pre-commit.
There was a problem hiding this comment.
will get this in near-future PR
| if ( op == ">=" ) return Op::GTE; | ||
| if ( op == "==" ) return Op::EQ; | ||
| if ( op == "!=" ) return Op::NEQ; | ||
| if ( op == "changed" ) return Op::CHANGED; // Could also use <> |
| */ | ||
| template <typename V1> | ||
| bool | ||
| cmp(V1 v, ObjectMapComparison::Op op, V1 w) |
There was a problem hiding this comment.
I would prefer to use C++ comparison functors like std::less, std::greater, std::equal rather than creating a separate ObjectMapComparison type, but that can be at a later release.
(Rev uses those functors to implement all of the different RISC-V comparisons.)
| long double v1 = static_cast<long double>(v); // unnecessary ... | ||
| long double w1 = static_cast<long double>(w); | ||
| return cmp(v1, op, w1); | ||
| } |
There was a problem hiding this comment.
This code can perhaps be made simpler with std::common_type_t, but that's for a later release.
| }; | ||
|
|
||
| std::string getName() { return name_; } | ||
| // TODO can we avoid code duplication in WatchPoint and the WatchPointAction? |
There was a problem hiding this comment.
Perhaps, with inheritance, delegation, a special class type, etc.
leekillough
left a comment
There was a problem hiding this comment.
Although I prefer printf "%.17g" (abstracted as printf "%.*g" with an argument) because of its precision, I approve this, in order to get it into 15.1.
We can work out the precision and representations later.
| } | ||
| } | ||
|
|
||
| // The business |
There was a problem hiding this comment.
Looks like half a comment?
|
|
||
| void setBufferReset(); | ||
| void check(); | ||
| uint32_t verbosity = 0; |
There was a problem hiding this comment.
Inconsistent code formatting. E.g., some vars camel case, some w/ trailing _, some elsewhere are _ separated. Should clean this up before the interactive console moves out of "experimental".
There was a problem hiding this comment.
Agreed - let's review the standard and we'll clean up all the code.
|
CLANG-FORMAT TEST - PASSED |
|
CLANG-FORMAT TEST - PASSED |
|
@gvoskuilen I pushed an update to include watchPoint.h in the distribution. This is to support work exploring an external tool using the interactive console. |
|
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; this inspection will remain valid until a new commit to source branch is performed. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MR-2
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MT-2
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-core_Make-Dist
Build InformationTest Name: SST__AutotestGen2_NewFW_OSX-14-XC15-ARM2_OMPI-4.1.6_PY3.10_sst-elements
Using Repos:
Pull Request Author: kpgriesser |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MR-2
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-elements_MT-2
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.9_sst-core_Make-Dist
Build InformationTest Name: SST__AutotestGen2_NewFW_OSX-14-XC15-ARM2_OMPI-4.1.6_PY3.10_sst-elements
|
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
feldergast
left a comment
There was a problem hiding this comment.
Looks good. We will likely want to revisit some of the ObjectMap structures as the APIs firm up.
| math/sqrt.h \ | ||
| uninitializedQueue.h \ | ||
| unitAlgebra.h \ | ||
| watchPoint.h \ |
There was a problem hiding this comment.
If watchPoint.h is added here, it should technically be removed from the sst_core_sources section.
| // information, see the LICENSE file in the top level directory of the | ||
| // distribution. | ||
|
|
||
| //clang-format off |
There was a problem hiding this comment.
Why is clang-format turned of here? They seem to be in the order specified in the .clang-format file.
There was a problem hiding this comment.
This is left-over from some earlier problems we had with include ordering and can be removed.
There was a problem hiding this comment.
By the way, the lack of a space between // and clang-format off prevents the directive from being effective. A space must exist between // and clang-format off in order for it to be effective. So leaving the code as-is without the space between // and clang-format off is benign and will just be treated as a comment.
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ feldergast ]! |
|
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge |
Major Changes:
config.h, config.cc
Add support for interactive realtime action, sst.rt.interactive (Interactive console realtime action #1266)
Add checkpoint-enable flag to enable checkpointing in interactive console
Make sst.interactive.simpledebug the default for interactive console
Only use --interactive-console to specify a custom IC File
replay option ( --replay )
from_string.h:
Full precision printing of float types. Scientific notation when value is greated than 106 or less than 10-6
impl/interactive/Makefile.inc
Included cmdLineEditor files for build
impl/interactive/cmdLineEditor.h, impl/interactive/cmdLineEditor.cc
In-line command editing support
impl/interactive/simpleDebug.h, impl/interactive/simpleDebug.cc
Shutdown support (Add support for shutdown from interactive console #1311) Watchpoint support
Add checkpoint-enabled flag to enable in interactive console
Make sst.interactive.simpledebug the default of interactive console
Only use --interactive-console to specify custom IC
Allow for empty command and other minor whitespace annoyances
Console command logging and replay. Support commented lines for replay scripts
Consolidated commands into a command funtion table.
Standardized specifying commands, help text, and command groups.
Provided auto-completion support for commands and object names.
Various nullptr checks and fixes to avoid segmentation faults.
interactiveConsole.h
Add support for shutdown (Add support for shutdown from interactive console #1311)
serialization/objectMap.cc, serialization/objectMap.h
Support for trace buffers and watchpoint actions.
Circular buffer and trigger recording Add ability to check/sample before and after both clock and event handlers.
Watchpoint support for adding set var val action.
Extended trigger tests functionality: support for logic operators (e.g. v1 < 10 && v2 > 5) and comparisons between two variables
Support for trigger comparison between two variables.
All set string to handle spaces. watchpoint verbosity control Various bug fixes for command line editing.
Fixes for nullptr checks and address sanitizer issues causing seg faults.
simulation.cc
Interactive console realtime action (Interactive console realtime action #1266).
Addition of checkpoint-enable flag to enable checkpoint in IC.
Make sst.interactive.simpledebug the default for IC replay option for IC
watchPoint.h, watchPoint.cc
Circular buffer, trigger record.
Add ability to check/sample before and after both clock and event handlers.
Add watchpoint action class and add set var val action.
Add support for trigger comparison between two variables Update watchpoint handling.
Allow set string to handle spaces
Add shutdown as trigger action.
Add watchpoint index to watchpoint class.
Add checkpoint-enable flag to enable checkpointing in interactive console.
Makesst.interactive.simpledebug the default for interactive console.
Only use --interactive-console to specify a custom IC.
watchpoint verbosity control.