Skip to content

Commit

Permalink
Fix annoying off by one bug that could cause collation to not happen in
Browse files Browse the repository at this point in the history
a timely fashion even if there were plenty of samples.
  • Loading branch information
jeremyestein committed Jan 22, 2025
1 parent 3c7df67 commit e77350c
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ private WaveformMessage collateContiguousData(SortedMap<Instant, WaveformMessage
// existing values are not necessarily in mutable lists so use a new ArrayList
List<Double> newNumericValues = new ArrayList<>();
Iterator<Map.Entry<Instant, WaveformMessage>> perPatientMapIter = perPatientMap.entrySet().iterator();
// keep track of incoming message sizes for general interest (does not affect collation algorithm)
Map<Integer, Integer> uncollatedMessageSizes = new HashMap<>();
int messagesToCollate = 0;
while (perPatientMapIter.hasNext()) {
Expand All @@ -178,13 +179,19 @@ private WaveformMessage collateContiguousData(SortedMap<Instant, WaveformMessage
int messageSizeCount = uncollatedMessageSizes.getOrDefault(thisMessageSampleCount, 0);
uncollatedMessageSizes.put(thisMessageSampleCount, messageSizeCount + 1);

if (sampleCount + thisMessageSampleCount > targetCollatedMessageSamples) {
logger.debug("Reached sample target ({} + {} > {}), collated message span: {} -> {}",
sampleCount, thisMessageSampleCount, targetCollatedMessageSamples,
// Ideally this code would be reworked, but for now it's important for sampleCount to meet or exceed
// targetCollatedMessageSamples even if we break out here (and thus don't include the current message).
// This is needed to prevent the check later on that would say the total sample count isn't big enough
// to make a message unless the data is old enough that we should collate regardless.
// Essentially, the later code is failing to realise that you can "meet" the target even if you're slightly
// short of it, because the next message would take us over the target.
sampleCount += thisMessageSampleCount;
if (sampleCount > targetCollatedMessageSamples) {
logger.debug("Reached sample target ({} > {}), collated message size {}, collated message span: {} -> {}",
sampleCount, targetCollatedMessageSamples, sampleCount - thisMessageSampleCount,
firstMsg.getObservationTime(), msg.getObservationTime());
break;
}
sampleCount += thisMessageSampleCount;

if (previousMsg != null) {
Instant expectedNextDatetime = previousMsg.getExpectedNextObservationDatetime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ private void makeAndAddTestMessages() throws WaveformCollator.CollationException
}

static Stream<Arguments> noGapsData() {
// We are adjusting the target number of samples config option rather than
// the actual number of samples supplied, which may be a bit unintuitive but
// is easier and amounts to the same thing.
// There is a fixed quantity of 3000 samples in messages containing 5 samples each.
// We are adjusting the *target* number of samples config option rather than
// the actual number of messages/samples supplied, which may be unintuitive but
// amounts to the same thing and means we can use the same test data each time.
return Stream.of(
// only just happened
Arguments.of(3000, 10000, List.of(3000), 0),
Expand Down

0 comments on commit e77350c

Please sign in to comment.