Skip to content

Commit a198859

Browse files
authored
Merge pull request #431 from GaneshPatil7517/fix/simtime-mutation
fix(simtime): remove global simtime mutation from write() across all languages (#385)
2 parents a283177 + 4e3a098 commit a198859

5 files changed

Lines changed: 151 additions & 6 deletions

File tree

concore.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ class Concore{
503503
outfile<<val[i]<<',';
504504
outfile<<val[val.size()-1]<<']';
505505
outfile.close();
506-
simtime += delta;
506+
// simtime must not be mutated here (issue #385).
507507
}
508508
else{
509509
throw 505;
@@ -559,7 +559,7 @@ class Concore{
559559
outfile<<val[val.size()-1]<<']';
560560
std::string result = outfile.str();
561561
std::strncpy(sharedData_create, result.c_str(), 256 - 1);
562-
simtime += delta;
562+
// simtime must not be mutated here (issue #385).
563563
}
564564
else{
565565
throw 505;

concore.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,8 @@ def write(port_identifier, name, val, delta=0):
382382
# Prepend simtime to match file-based write behavior
383383
payload = [simtime + delta] + zmq_val
384384
zmq_p.send_json_with_retry(payload)
385-
simtime += delta
385+
# simtime must not be mutated here.
386+
# Mutation breaks cross-language determinism (see issue #385).
386387
else:
387388
zmq_p.send_json_with_retry(zmq_val)
388389
except zmq.error.ZMQError as e:
@@ -413,7 +414,8 @@ def write(port_identifier, name, val, delta=0):
413414
val_converted = convert_numpy_to_python(val)
414415
data_to_write = [simtime + delta] + val_converted
415416
outfile.write(str(data_to_write))
416-
simtime += delta
417+
# simtime must not be mutated here.
418+
# Mutation breaks cross-language determinism (see issue #385).
417419
else:
418420
outfile.write(val)
419421
except Exception as e:

concore.v

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ module concore;
296296
end
297297
$fdisplay(fout,"]");
298298
$fclose(fout);
299-
simtime = simtime + delta;
299+
// simtime must not be mutated here (issue #385).
300300
end
301301
endtask
302302

concore_write.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ function concore_write(port, name, val, delta)
55
outstr = cat(2,"[",num2str(concore.simtime+delta),num2str(val,",%e"),"]");
66
fprintf(output1,'%s',outstr);
77
fclose(output1);
8-
concore.simtime = concore.simtime + delta;
8+
% simtime must not be mutated here (issue #385).
99
catch exc
1010
disp(['skipping ' concore.outpath num2str(port) '/' name]);
1111
end

tests/test_concore.py

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,3 +276,146 @@ def recv_json_with_retry(self):
276276
# Read should return original data (simtime stripped)
277277
result = concore.read("roundtrip_test", "data", "[]")
278278
assert result == original_data
279+
280+
281+
class TestSimtimeNotMutatedByWrite:
282+
"""Regression tests for issue #385:
283+
write() must NOT mutate global simtime. Simtime advancement happens
284+
only in read() via max(simtime, file_simtime). Mutating simtime in
285+
write() causes cascading timestamps in multi-output-port nodes and
286+
breaks cross-language determinism.
287+
"""
288+
289+
@pytest.fixture(autouse=True)
290+
def reset_simtime(self):
291+
import concore
292+
old_simtime = concore.simtime
293+
yield
294+
concore.simtime = old_simtime
295+
296+
@pytest.fixture(autouse=True)
297+
def reset_outpath(self):
298+
import concore
299+
old_outpath = concore.outpath
300+
yield
301+
concore.outpath = old_outpath
302+
303+
@pytest.fixture(autouse=True)
304+
def reset_zmq_ports(self):
305+
import concore
306+
original_ports = concore.zmq_ports.copy()
307+
yield
308+
concore.zmq_ports.clear()
309+
concore.zmq_ports.update(original_ports)
310+
311+
# ---- Test Case 1: single-output write keeps simtime unchanged ----
312+
313+
def test_single_file_write_does_not_mutate_simtime(self, temp_dir):
314+
"""A single file-based write with delta must not change simtime."""
315+
import concore
316+
317+
concore.simtime = 10
318+
out_dir = os.path.join(temp_dir, "out1")
319+
os.makedirs(out_dir, exist_ok=True)
320+
concore.outpath = os.path.join(temp_dir, "out")
321+
322+
concore.write(1, "v", [5.0], delta=1)
323+
324+
assert concore.simtime == 10, (
325+
"simtime must not be mutated by write(); "
326+
"was %s instead of 10" % concore.simtime
327+
)
328+
329+
def test_single_zmq_write_does_not_mutate_simtime(self):
330+
"""A single ZMQ-based write with delta must not change simtime."""
331+
import concore
332+
333+
class DummyPort:
334+
def send_json_with_retry(self, msg):
335+
self.sent = msg
336+
337+
dummy = DummyPort()
338+
concore.zmq_ports["zmq_test"] = dummy
339+
concore.simtime = 10
340+
341+
concore.write("zmq_test", "v", [5.0], delta=1)
342+
343+
assert concore.simtime == 10, (
344+
"simtime must not be mutated by ZMQ write(); "
345+
"was %s instead of 10" % concore.simtime
346+
)
347+
348+
# ---- Test Case 2: multi-port write → identical timestamps ----
349+
350+
def test_multi_port_file_writes_share_same_timestamp(self, temp_dir):
351+
"""Two consecutive file writes with delta=1 must produce the
352+
same timestamp (simtime+delta), proving simtime is not incremented
353+
between calls."""
354+
import concore
355+
356+
concore.simtime = 10
357+
concore.outpath = os.path.join(temp_dir, "out")
358+
for p in (1, 2):
359+
os.makedirs(os.path.join(temp_dir, "out" + str(p)), exist_ok=True)
360+
361+
concore.write(1, "u", [1.0], delta=1)
362+
concore.write(2, "v", [2.0], delta=1)
363+
364+
# Read back the written files and compare timestamps
365+
from ast import literal_eval
366+
payloads = []
367+
for p in (1, 2):
368+
with open(os.path.join(temp_dir, "out" + str(p),
369+
("u" if p == 1 else "v"))) as f:
370+
payloads.append(literal_eval(f.read()))
371+
372+
ts1, ts2 = payloads[0][0], payloads[1][0]
373+
assert ts1 == ts2 == 11, (
374+
"Both ports must share timestamp simtime+delta=11; "
375+
"got %s and %s" % (ts1, ts2)
376+
)
377+
378+
def test_multi_port_zmq_writes_share_same_timestamp(self):
379+
"""Two consecutive ZMQ writes with delta=1 must produce the
380+
same timestamp."""
381+
import concore
382+
383+
class DummyPort:
384+
def __init__(self):
385+
self.sent = None
386+
def send_json_with_retry(self, msg):
387+
self.sent = msg
388+
389+
d1, d2 = DummyPort(), DummyPort()
390+
concore.zmq_ports["p1"] = d1
391+
concore.zmq_ports["p2"] = d2
392+
concore.simtime = 10
393+
394+
concore.write("p1", "u", [1.0], delta=1)
395+
concore.write("p2", "v", [2.0], delta=1)
396+
397+
assert d1.sent[0] == d2.sent[0] == 11, (
398+
"Both ZMQ ports must share timestamp 11; got %s and %s"
399+
% (d1.sent[0], d2.sent[0])
400+
)
401+
402+
# ---- Test Case 3: cross-language parity check ----
403+
404+
def test_write_timestamp_matches_cpp_semantics(self, temp_dir):
405+
"""C++ uses `simtime+delta` as a local expression without mutation.
406+
After N writes with delta=1, simtime must still be the original
407+
value — matching C++ behaviour."""
408+
import concore
409+
410+
concore.simtime = 0
411+
concore.outpath = os.path.join(temp_dir, "out")
412+
for p in range(1, 4):
413+
os.makedirs(os.path.join(temp_dir, "out" + str(p)), exist_ok=True)
414+
415+
for p in range(1, 4):
416+
concore.write(p, "x", [float(p)], delta=1)
417+
418+
assert concore.simtime == 0, (
419+
"After 3 writes with delta=1 simtime must remain 0 "
420+
"(matching C++/MATLAB/Verilog); got %s" % concore.simtime
421+
)

0 commit comments

Comments
 (0)