Skip to content

[Medium] CSV row newline only written inside the double-observables loop, dropped when a hit has no doubles #129

@zhaozhiwen

Description

@zhaozhiwen

one-line summary
The CSV row terminator ("\n") is emitted only on the final iteration of the double-valued observables loop; a hit with zero double observables never enters the loop body, so its row gets no newline and merges with the next row.

Affected files: gemc/gstreamer/factories/CSV/event/publishDigitized.cc:54-58, gemc/gstreamer/factories/CSV/event/publishTrueInfo.cc:55-59, gemc/gstreamer/factories/CSV/run/publishDigitized.cc:55-59

Details: In all three files the per-hit row is written as int observables (each followed by ", ") then double observables, with the newline buried in the else branch of the double loop:

// event/publishDigitized.cc
size_t total = dmap.size();
size_t i     = 0;
ofile_digitized << event_number << ", " << ... << detectorName << ", ";
for (const auto& [variableName, value] : imap) { ofile_digitized << value << ", "; }
for (const auto& [variableName, value] : dmap) {
    ofile_digitized << value;
    if (++i < total) ofile_digitized << ", ";
    else ofile_digitized << "\n";   // <-- only reached if dmap is non-empty
}

When dmap.empty() (total == 0), the loop body never executes, so neither the ", "/"\n" separators run. The row ends with a trailing ", " from the int loop and no newline, so the next hit's row is concatenated onto the same line. The same pattern (newline inside the dmap/double loop) appears in publishTrueInfo.cc:55-59 and run/publishDigitized.cc:55-59.

Impact: Any digitized or true-info hit whose detector schema has only integer observables (no double observables) corrupts the CSV: that row and the following row share a line, and an extra trailing separator appears. Affects the csv gstreamer for such schemas in both event and run mode.

Proposed fix: Drop the embedded terminator and write the row newline exactly once per hit, after the double loop. Apply the same change to all three files. For event/publishDigitized.cc:

 		for (const auto& [variableName, value] : imap) { ofile_digitized << value << ", "; }
-		for (const auto& [variableName, value] : dmap) {
-			ofile_digitized << value;
-			if (++i < total) ofile_digitized << ", ";
-			else ofile_digitized << "\n";
-		}
+		for (const auto& [variableName, value] : dmap) {
+			ofile_digitized << value;
+			if (++i < total) ofile_digitized << ", ";
+		}
+		ofile_digitized << "\n";

For run/publishDigitized.cc the change is identical (stream is ofile_digitized). For event/publishTrueInfo.cc the stream is ofile_true_info:

 		for (const auto& [variableName, value] : smap) { ofile_true_info << value << ", "; }
-		for (const auto& [variableName, value] : dmap) {
-			ofile_true_info << value;
-			if (++i < total) ofile_true_info << ", ";
-			else ofile_true_info << "\n";
-		}
+		for (const auto& [variableName, value] : dmap) {
+			ofile_true_info << value;
+			if (++i < total) ofile_true_info << ", ";
+		}
+		ofile_true_info << "\n";

Note on a residual edge case: when dmap is empty the int loop still leaves a trailing ", " before the newline (an empty last column). That matches the existing header-row behavior (the header also appends ", " after the int names and only the dmap names are tail-trimmed), so the column count stays consistent. Tightening that trailing separator is optional and orthogonal to the missing-newline bug fixed here.


Split out from #102 (one issue per bug). Found via an AI-assisted code review with manual verification against commit 5f8ce875.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions