Resolve #5 #13
Resolve #5 #13
Conversation
MasWag
left a comment
MasWag
left a comment
There was a problem hiding this comment.
I find that the intention of some of the comments are unclear. Can you clarify or fix them?
test/data_parametric_monitor_test.cc
Outdated
|
|
||
| using TWEvent = TimedWordEvent<PPLRational>; | ||
|
|
||
| //DummyTimedWordSubject だと DummyTimedWordSubject2{std::move(vec)}.addObserver(monitor); |
There was a problem hiding this comment.
This error is because DummyTimedWordSubject is already defined in test/boolean_monitor_test.cc
There was a problem hiding this comment.
I guessed as you said.
Renamed DummyTimedWordSubject2 to DummyDataTimedWordSubject
test/data_parametric_monitor_test.cc
Outdated
| #include <boost/mpl/list.hpp> | ||
| #include "../src/data_parametric_monitor.hh" | ||
| #include "../test/fixture/copy_automaton_fixture.hh" | ||
| //TODO: copy_automaton_fixture の automaton は NonParametricTA だが、DataParametricTA にすべきな気がする |
There was a problem hiding this comment.
The fixture of NonParametricTA is used in boolean_monitor_test.cc
There was a problem hiding this comment.
(What was this line...?)
Just removed it.
* Remove memo * DummyTimedWordSubject to DummyDataTimedWordSubject for clarity and consistency
|
Oops. Sorry for forgetting to remove my memo |
There was a problem hiding this comment.
Pull request overview
This PR updates the DataParametricMonitor to handle rational timestamps and data values by replacing Parma_Polyhedra_Library::Coefficient (integers) with PPLRational (rational numbers represented as numerator/denominator pairs).
Key changes:
- Replaces
Coefficienttype withPPLRationalin DataParametricMonitor and main.cc - Updates constraint handling to convert rational constraints to integer-based constraints using the formula
var * denominator == numerator - Adds comprehensive test coverage for the updated DataParametricMonitor with rational number inputs
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/data_parametric_monitor.hh | Updated type signatures from Coefficient to PPLRational and modified constraint addition to handle rational numbers by multiplying by denominator |
| src/main.cc | Replaced Coefficient type alias with direct PPLRational usage in execute calls for dataparametric mode |
| test/data_parametric_monitor_test.cc | Added new test file with test fixtures and two test cases to verify DataParametricMonitor behavior with rational number inputs |
| CMakeLists.txt | Added the new test file to the build configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2 participants
Update DataParametricMonitor to handle rational timestamp by simply replace some
Parma_Polyhedra_Library::CoefficientwithPPLRational(#5)