Repository navigation
Conversation
Since ControlCore-Project#431 write() no longer touches simtime, so a node only moves its clock when it reads. A plant that writes with delta=1 never reaches maxtime by itself any more. If its controller finishes first and writes its init values back to the port before the plant has read the last real value, the plant reads the reset value, stays one step short of maxtime and waits forever (ControlCore-Project#608). It shows up when the plant is a bit slower than the controller, e.g. on Windows or with MATLAB/Verilog. write() still leaves simtime alone, so writes in the same step keep the same timestamp (ControlCore-Project#385). It now remembers simtime + delta, and the next read moves simtime up to that before reading. If simtime was changed in between (initval, or set by hand) the pending advance is dropped. Done the same way in Python, C++ (concore.hpp, concoredocker.hpp), Java, MATLAB/Octave and Julia (concore.jl, concoredocker.jl), with a test in each. tests/test_end_of_run.py runs a controller and a slower plant as two processes and hangs without this change. Verilog needs ControlCore-Project#604 first since readdata still overwrites simtime there. Fixes ControlCore-Project#608
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #608
Since #431
write()doesn't touchsimtime, so a node only moves its clock when it reads. A plant that writes withdelta=1can't reach maxtime by itself anymore, and if the controller finishes first and writes its init values back to the port before the plant has read the last real value, the plant gets stuck one step before the end. More details and the repro are in #608.I went with the first option from the issue:
write()still doesn't changesimtime, so writes in the same step keep the same timestamp and thesimtimemutation semantics diverge across language implementations, causing non-deterministic multi-port write behavior #385 tests stay as they aresimtime + delta, and the next read movessimtimeup to that before readingsimtimewas changed in between (initvalor set by hand) the pending advance is droppedDuring a normal run this gives the same simtime as before since the next read would have brought it there anyway. The only difference is at the very end, where the slower node can do one more step with the reset value before it stops, instead of hanging.
Done the same way in Python (
concore_base.py), C++ (concore.hpp,concoredocker.hpp), Java, MATLAB/Octave and Julia (concore.jl,concoredocker.jl), each with a small test that fails on dev:tests/test_end_of_run.pyruns a controller and a slower plant as two processes, same shape astestsou/cpymat.py/pmpymat.py. On dev the plant never finishes, with this it passes in ~2stests/test_concore.py,TestConcoreHpp.cpp,TestConcoreApi.java,tests/julia/test_protocol.jland a newtests/matlab/test_write_advance.m(added to the existing MATLAB step in CI)The phase 1 conformance runner now clears the pending write before the read cases, since those cases expect a fresh node.
Verilog isn't in here yet.
readdatainconcore.vstill overwritessimtimewith whatever it reads, so this would be undone right away. That's fixed in #604, I'll add the Verilog part once that's in.Before
The new test against dev:
After