From 40ee4f511bf547c0432c5f712275b9d706420919 Mon Sep 17 00:00:00 2001 From: Gabriel Mechali Date: Fri, 14 Aug 2026 13:23:42 -0400 Subject: [PATCH 1/5] Preserve SigFigs through Dataflow. Stop converting to double/int --- .../ingestion/util/GraphReader.java | 8 ++-- .../ingestion/util/GraphReaderTest.java | 20 ++++++++++ .../ingestion/util/PipelineUtilsTest.java | 40 ++++++++++++++++--- .../java/org/datacommons/util/GraphUtils.java | 38 +++++------------- .../org/datacommons/util/StatChecker.java | 14 ++----- .../org/datacommons/util/GraphUtilsTest.java | 37 +++++++++++++++-- .../org/datacommons/util/StatCheckerTest.java | 2 +- 7 files changed, 109 insertions(+), 50 deletions(-) diff --git a/pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphReader.java b/pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphReader.java index 12758889e..10b4948df 100644 --- a/pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphReader.java +++ b/pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphReader.java @@ -496,10 +496,12 @@ static TimeSeriesKey toTimeSeriesKey(McfStatVarObsSeries.Key key, String importN static Observation toObservation(TimeSeriesKey seriesKey, StatVarObs obs) { String value = ""; - if (obs.hasNumber()) { - value = Double.toString(obs.getNumber()); - } else if (obs.hasText()) { + // Prefer text representation to preserve significant figures. + // Fall back to legacy number field for backward compatibility with older serialized protos. + if (obs.hasText()) { value = obs.getText(); + } else if (obs.hasNumber()) { + value = Double.toString(obs.getNumber()); } return Observation.builder().seriesKey(seriesKey).date(obs.getDate()).value(value).build(); diff --git a/pipeline/util/src/test/java/org/datacommons/ingestion/util/GraphReaderTest.java b/pipeline/util/src/test/java/org/datacommons/ingestion/util/GraphReaderTest.java index b0624131a..17c1a2556 100644 --- a/pipeline/util/src/test/java/org/datacommons/ingestion/util/GraphReaderTest.java +++ b/pipeline/util/src/test/java/org/datacommons/ingestion/util/GraphReaderTest.java @@ -389,6 +389,14 @@ public void testToObservation() { StatVarObs.newBuilder().setDcid("obs1").setDate("2020").setNumber(10.0).build(); StatVarObs obs2 = StatVarObs.newBuilder().setDcid("obs2").setDate("2021").setText("someText").build(); + StatVarObs obs3 = + StatVarObs.newBuilder().setDcid("obs3").setDate("2022").setText("12.5000").build(); + StatVarObs obs4 = + StatVarObs.newBuilder() + .setDcid("obs4") + .setDate("2023") + .setText("100000000000000001") + .build(); Observation expected1 = Observation.builder().seriesKey(seriesKey).date("2020").value("10.0").build(); @@ -396,11 +404,23 @@ public void testToObservation() { Observation expected2 = Observation.builder().seriesKey(seriesKey).date("2021").value("someText").build(); + Observation expected3 = + Observation.builder().seriesKey(seriesKey).date("2022").value("12.5000").build(); + + Observation expected4 = + Observation.builder().seriesKey(seriesKey).date("2023").value("100000000000000001").build(); + Observation actual1 = GraphReader.toObservation(seriesKey, obs1); assertEquals(expected1, actual1); Observation actual2 = GraphReader.toObservation(seriesKey, obs2); assertEquals(expected2, actual2); + + Observation actual3 = GraphReader.toObservation(seriesKey, obs3); + assertEquals(expected3, actual3); + + Observation actual4 = GraphReader.toObservation(seriesKey, obs4); + assertEquals(expected4, actual4); } @Test diff --git a/pipeline/util/src/test/java/org/datacommons/ingestion/util/PipelineUtilsTest.java b/pipeline/util/src/test/java/org/datacommons/ingestion/util/PipelineUtilsTest.java index e7a5dae4e..c638b1343 100644 --- a/pipeline/util/src/test/java/org/datacommons/ingestion/util/PipelineUtilsTest.java +++ b/pipeline/util/src/test/java/org/datacommons/ingestion/util/PipelineUtilsTest.java @@ -62,10 +62,10 @@ private McfGraph createStatVarObservationGraph( return graph.build(); } - private McfStatVarObsSeries.StatVarObs createStatVarObs(String date, double value, String dcid) { + private McfStatVarObsSeries.StatVarObs createStatVarObs(String date, String value, String dcid) { McfStatVarObsSeries.StatVarObs.Builder svObs = McfStatVarObsSeries.StatVarObs.newBuilder(); svObs.setDate(date); - svObs.setNumber(value); + svObs.setText(value); svObs.setDcid(dcid); svObs.setPvs(PropertyValues.newBuilder().build()); return svObs.build(); @@ -114,8 +114,8 @@ public void testBuildOptimizedMcfGraph() { "count_person", "country/USA", Arrays.asList( - createStatVarObs("2020", 32.0, "obs1"), - createStatVarObs("2021", 33.0, "obs2")))) + createStatVarObs("2020", "32.0", "obs1"), + createStatVarObs("2021", "33.0", "obs2")))) .build(); McfOptimizedGraph expected2 = McfOptimizedGraph.newBuilder() @@ -123,7 +123,7 @@ public void testBuildOptimizedMcfGraph() { createMcfStatVarObsSeries( "count_person", "country/India", - List.of(createStatVarObs("2022", 36.0, "obs4")))) + List.of(createStatVarObs("2022", "36.0", "obs4")))) .build(); PAssert.that(result).containsInAnyOrder(expected1, expected2); @@ -131,6 +131,36 @@ public void testBuildOptimizedMcfGraph() { Assert.assertEquals(PipelineResult.State.DONE, state); } + @Test + public void testBuildOptimizedMcfGraph_preservesSigFigs() { + options.setStableUniqueNames(PipelineOptions.CheckEnabled.OFF); + p.getCoderRegistry() + .registerCoderForClass( + McfStatVarObsSeries.Key.class, ProtoCoder.of(McfStatVarObsSeries.Key.class)); + + PCollection input = + p.apply( + Create.of( + createStatVarObservationGraph( + "obsSigFig", "measurement_rate", "country/USA", "2020", "12.5000"))); + + PCollection result = + PipelineUtils.buildOptimizedMcfGraph("testSigFig", input); + + McfOptimizedGraph expected = + McfOptimizedGraph.newBuilder() + .setSvObsSeries( + createMcfStatVarObsSeries( + "measurement_rate", + "country/USA", + List.of(createStatVarObs("2020", "12.5000", "obsSigFig")))) + .build(); + + PAssert.that(result).containsInAnyOrder(expected); + PipelineResult.State state = p.run().waitUntilFinish(); + Assert.assertEquals(PipelineResult.State.DONE, state); + } + @Test public void testCombineGraphNodes() { // Input Graph 1 diff --git a/util/src/main/java/org/datacommons/util/GraphUtils.java b/util/src/main/java/org/datacommons/util/GraphUtils.java index 7bcad52fe..7fcffb60c 100644 --- a/util/src/main/java/org/datacommons/util/GraphUtils.java +++ b/util/src/main/java/org/datacommons/util/GraphUtils.java @@ -191,25 +191,7 @@ public static List getPropertyValues(Map pvs, S } /** - * Gets the double value of a specific property from a graph node. - * - * @param node The graph node (PropertyValues) to read from. - * @param prop The property name whose value should be retrieved as a double. - * @return The double value of the property, or Double.NaN if the value is not a valid number. - */ - public static Double nodeDoubleValue(McfGraph.PropertyValues node, String prop) { - String str_val = getPropVal(node, prop); - if (str_val.isEmpty()) throw new IllegalArgumentException("Failed to get double value."); - try { - double v = Double.parseDouble(str_val); - return v; - } catch (NumberFormatException nfe) { - return Double.NaN; - } - } - - /** - * Flattens an optimized MCF graph into a list of graph node + * Flattens an optimized MCF graph into a list of graph nodes. * * @param optimized_graph input optimized graph * @return list of McfGraph instances, each representing a single StatVarObservation. @@ -256,10 +238,14 @@ public static List convertMcfStatVarObsSeriesToMcfGraph( // Set required PVs. setPropVal(Property.dcid.name(), ValueType.TEXT, o.getDcid(), node); setPropVal(Property.observationDate.name(), ValueType.TEXT, o.getDate(), node); - if (o.hasNumber()) { + // Prefer text representation to preserve significant figures (SigFigs). + // Fall back to legacy number field for backward compatibility with older serialized protos. + if (o.hasText()) { + String valText = o.getText(); + ValueType valType = StringUtil.isNumber(valText) ? ValueType.NUMBER : ValueType.TEXT; + setPropVal(Property.value.name(), valType, valText, node); + } else if (o.hasNumber()) { setPropVal(Property.value.name(), ValueType.NUMBER, Double.toString(o.getNumber()), node); - } else if (o.hasText()) { - setPropVal(Property.value.name(), ValueType.TEXT, o.getText(), node); } // Set optional PVs. @@ -329,12 +315,8 @@ public static McfStatVarObsSeries convertMcfGraphToMcfStatVarObsSeries( if (!useDcidForLocalNodeIdInOptimizedMcf(dcid, nodeId)) { svo.setLocalNodeId(nodeId); } - Double value; - if (!(value = nodeDoubleValue(node, "value")).isNaN()) { - svo.setNumber(value); - } else { // Non-number value. - svo.setText(getPropVal(node, "value")); - } + // Preserve raw string representation to maintain significant figures (SigFigs). + svo.setText(getPropVal(node, "value")); McfGraph.PropertyValues.Builder pvs = svo.getPvsBuilder(); for (Map.Entry entry : node.getPvsMap().entrySet()) { String prop = entry.getKey(); diff --git a/util/src/main/java/org/datacommons/util/StatChecker.java b/util/src/main/java/org/datacommons/util/StatChecker.java index 271a6eb40..00c4bc9a4 100644 --- a/util/src/main/java/org/datacommons/util/StatChecker.java +++ b/util/src/main/java/org/datacommons/util/StatChecker.java @@ -81,10 +81,9 @@ public class StatChecker { private final ConcurrentMap> samplePlaces; private final boolean shouldGenerateSamplePlaces; // Tracks global state on StatVarObservations to detect whether there are multiple of the - // same StatVarObservation with inconsistent values. The key is a hash made up of a set of - // properties that distinguish a StatVarObservation and the value is the first value seen of that - // StatVarObservation. - private final ConcurrentMap svObValues; + // same StatVarObservation with inconsistent values. Stored as raw String to preserve + // significant figures (SigFigs) and avoid floating-point precision collisions. + private final ConcurrentMap svObValues; private final String EMPTY_PROP_STRING = "EMPTY_PROP"; private StatVarState statVarState; private ExistenceChecker existenceChecker; @@ -604,12 +603,7 @@ private boolean checkSvObsValueInconsistency(McfGraph.PropertyValues node) { } } Long fp = hasher.hash().asLong(); - Float val = null; - try { - val = Float.parseFloat(McfUtil.getPropVal(node, Vocabulary.VALUE)); - } catch (NumberFormatException e) { - // If value is not a float, val will stay as null and this will be handled later. - } + String val = McfUtil.getPropVal(node, Vocabulary.VALUE); if (this.svObValues.containsKey(fp) && !this.svObValues.get(fp).equals(val)) { logCtx.addEntry( Level.LEVEL_ERROR, diff --git a/util/src/test/java/org/datacommons/util/GraphUtilsTest.java b/util/src/test/java/org/datacommons/util/GraphUtilsTest.java index 04c82614c..fa60a3a4c 100644 --- a/util/src/test/java/org/datacommons/util/GraphUtilsTest.java +++ b/util/src/test/java/org/datacommons/util/GraphUtilsTest.java @@ -107,7 +107,7 @@ public void testBuildOptimizedMcfGraph_singleObservation() { McfStatVarObsSeries.StatVarObs obs = series.getSvObsList(0); assertEquals("2023-01-15", obs.getDate()); assertEquals("obs1", obs.getDcid()); - assertEquals(100.5, obs.getNumber(), 0.001); + assertEquals("100.5", obs.getText()); assertEquals("A test observation", GraphUtils.getPropVal(obs.getPvs(), "description")); } @@ -169,12 +169,12 @@ public void testBuildOptimizedMcfGraph_multipleObservations_sameKey() { McfStatVarObsSeries.StatVarObs obs1 = series.getSvObsList(0); assertEquals("2023-01-15", obs1.getDate()); assertEquals("obs1", obs1.getDcid()); - assertEquals(100.5, obs1.getNumber(), 0.001); + assertEquals("100.5", obs1.getText()); McfStatVarObsSeries.StatVarObs obs2 = series.getSvObsList(1); assertEquals("2023-02-15", obs2.getDate()); assertEquals("obs2", obs2.getDcid()); - assertEquals(102.0, obs2.getNumber(), 0.001); + assertEquals("102.0", obs2.getText()); } @Test @@ -216,4 +216,35 @@ public void testBuildOptimizedMcfGraph_multipleObservations_differentKeys() { assertNotNull(result); assertEquals(2, result.size()); } + + @Test + public void testBuildOptimizedMcfGraph_preservesSigFigsAndLargeNumbers() { + McfGraph.PropertyValues svoNode = + createSVObsNode( + "obsSigFig", + "dcid:placeA", + "dcid:svPrecision", + "2023-01-15", + "12.5000", + ValueType.NUMBER, + "P1M", + "dcid:methodA", + "dcid:unitX", + "1", + null); + + McfGraph mcfGraph = + McfGraph.newBuilder().setType(McfType.INSTANCE_MCF).putNodes("l:node1", svoNode).build(); + + List result = GraphUtils.buildOptimizedMcfGraph(List.of(mcfGraph)); + assertEquals(1, result.size()); + McfStatVarObsSeries.StatVarObs obs = result.get(0).getSvObsSeries().getSvObsList(0); + assertEquals("12.5000", obs.getText()); + + // Also verify roundtrip back to McfGraph + List roundTripGraphs = GraphUtils.convertMcfStatVarObsSeriesToMcfGraph(result.get(0)); + assertEquals(1, roundTripGraphs.size()); + McfGraph.PropertyValues node = roundTripGraphs.get(0).getNodesOrThrow("l:node1"); + assertEquals("12.5000", GraphUtils.getPropVal(node, "value")); + } } diff --git a/util/src/test/java/org/datacommons/util/StatCheckerTest.java b/util/src/test/java/org/datacommons/util/StatCheckerTest.java index 711fd1443..ba2567c7a 100644 --- a/util/src/test/java/org/datacommons/util/StatCheckerTest.java +++ b/util/src/test/java/org/datacommons/util/StatCheckerTest.java @@ -125,7 +125,7 @@ public void testCheckSvObsInGraph() throws IOException { TestUtil.checkLog( lw.getLog(), "Sanity_InconsistentSvObsValues", - "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/SF', variableMeasured: 'WomenIncome', observationDate: '2020', value1: 1.0E7, value2: 1.0000001E7")); + "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/SF', variableMeasured: 'WomenIncome', observationDate: '2020', value1: 10000000.0, value2: 10000001.0")); // check node that differs only in one property from an existing StatVarObservation node. mcf = From 4d81044d11c8a5170ea0ab41d2301e7962642024 Mon Sep 17 00:00:00 2001 From: Gabriel Mechali Date: Fri, 14 Aug 2026 14:36:41 -0400 Subject: [PATCH 2/5] Remove the MCF obs number bc we removed all usages of it. Reserve the field in proto --- .../org/datacommons/ingestion/util/GraphReader.java | 10 +--------- .../datacommons/ingestion/util/GraphReaderTest.java | 2 +- .../main/java/org/datacommons/util/GraphUtils.java | 4 ---- util/src/main/proto/Mcf.proto | 11 +++++------ 4 files changed, 7 insertions(+), 20 deletions(-) diff --git a/pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphReader.java b/pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphReader.java index 10b4948df..0ac328e51 100644 --- a/pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphReader.java +++ b/pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphReader.java @@ -495,15 +495,7 @@ static TimeSeriesKey toTimeSeriesKey(McfStatVarObsSeries.Key key, String importN } static Observation toObservation(TimeSeriesKey seriesKey, StatVarObs obs) { - String value = ""; - // Prefer text representation to preserve significant figures. - // Fall back to legacy number field for backward compatibility with older serialized protos. - if (obs.hasText()) { - value = obs.getText(); - } else if (obs.hasNumber()) { - value = Double.toString(obs.getNumber()); - } - + String value = obs.hasText() ? obs.getText() : ""; return Observation.builder().seriesKey(seriesKey).date(obs.getDate()).value(value).build(); } diff --git a/pipeline/util/src/test/java/org/datacommons/ingestion/util/GraphReaderTest.java b/pipeline/util/src/test/java/org/datacommons/ingestion/util/GraphReaderTest.java index 17c1a2556..56d1bce56 100644 --- a/pipeline/util/src/test/java/org/datacommons/ingestion/util/GraphReaderTest.java +++ b/pipeline/util/src/test/java/org/datacommons/ingestion/util/GraphReaderTest.java @@ -386,7 +386,7 @@ public void testToObservation() { "testStatVar", "geoId/testPlace", "", "P1Y", "testMethod", "testUnit", "100", facetId); StatVarObs obs1 = - StatVarObs.newBuilder().setDcid("obs1").setDate("2020").setNumber(10.0).build(); + StatVarObs.newBuilder().setDcid("obs1").setDate("2020").setText("10.0").build(); StatVarObs obs2 = StatVarObs.newBuilder().setDcid("obs2").setDate("2021").setText("someText").build(); StatVarObs obs3 = diff --git a/util/src/main/java/org/datacommons/util/GraphUtils.java b/util/src/main/java/org/datacommons/util/GraphUtils.java index 7fcffb60c..73180a5c0 100644 --- a/util/src/main/java/org/datacommons/util/GraphUtils.java +++ b/util/src/main/java/org/datacommons/util/GraphUtils.java @@ -238,14 +238,10 @@ public static List convertMcfStatVarObsSeriesToMcfGraph( // Set required PVs. setPropVal(Property.dcid.name(), ValueType.TEXT, o.getDcid(), node); setPropVal(Property.observationDate.name(), ValueType.TEXT, o.getDate(), node); - // Prefer text representation to preserve significant figures (SigFigs). - // Fall back to legacy number field for backward compatibility with older serialized protos. if (o.hasText()) { String valText = o.getText(); ValueType valType = StringUtil.isNumber(valText) ? ValueType.NUMBER : ValueType.TEXT; setPropVal(Property.value.name(), valType, valText, node); - } else if (o.hasNumber()) { - setPropVal(Property.value.name(), ValueType.NUMBER, Double.toString(o.getNumber()), node); } // Set optional PVs. diff --git a/util/src/main/proto/Mcf.proto b/util/src/main/proto/Mcf.proto index eef1ba67c..211a98b18 100644 --- a/util/src/main/proto/Mcf.proto +++ b/util/src/main/proto/Mcf.proto @@ -99,14 +99,13 @@ enum ValueType { // This is effectively similar to SourceSeries, but with free PVs per SVObs. message McfStatVarObsSeries { message StatVarObs { + reserved 2; + reserved "number"; + required string date = 1; - oneof val { - // Vast majority of values are numbers, storing them as double will be - // more space efficient. - double number = 2; - string text = 3; - } + // Value of the observation in text format. + optional string text = 3; // Any additional PVs associated with the StatVarObs. optional McfGraph.PropertyValues pvs = 4; From c40baa0f9c7a5ef6bd0dee420125f9dd10009ecf Mon Sep 17 00:00:00 2001 From: Gabriel Mechali Date: Fri, 14 Aug 2026 14:49:05 -0400 Subject: [PATCH 3/5] Report updates --- .../manyinconsistent/output/report.json | 4 +- .../tool/genmcf/statchecks/output/report.json | 2 +- .../tool/lint/statchecks/output/report.json | 245 ++++++++++-------- 3 files changed, 145 insertions(+), 106 deletions(-) diff --git a/tool/src/test/resources/org/datacommons/tool/genmcf/manyinconsistent/output/report.json b/tool/src/test/resources/org/datacommons/tool/genmcf/manyinconsistent/output/report.json index 9d139774d..405a92e08 100644 --- a/tool/src/test/resources/org/datacommons/tool/genmcf/manyinconsistent/output/report.json +++ b/tool/src/test/resources/org/datacommons/tool/genmcf/manyinconsistent/output/report.json @@ -28,7 +28,7 @@ "file": "acre.csv", "lineNumber": "3" }, - "userMessage": "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/01', variableMeasured: 'Some_StatVar', observationDate: '2020-01-01', value1: 1.0, value2: 2.0", + "userMessage": "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/01', variableMeasured: 'Some_StatVar', observationDate: '2020-01-01', value1: 1, value2: 2", "counterKey": "Sanity_InconsistentSvObsValues" }, { "level": "LEVEL_ERROR", @@ -36,7 +36,7 @@ "file": "acre.csv", "lineNumber": "4" }, - "userMessage": "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/01', variableMeasured: 'Some_StatVar', observationDate: '2020-01-01', value1: 1.0, value2: 3.0", + "userMessage": "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/01', variableMeasured: 'Some_StatVar', observationDate: '2020-01-01', value1: 1, value2: 3", "counterKey": "Sanity_InconsistentSvObsValues" }, { "level": "LEVEL_ERROR", diff --git a/tool/src/test/resources/org/datacommons/tool/genmcf/statchecks/output/report.json b/tool/src/test/resources/org/datacommons/tool/genmcf/statchecks/output/report.json index 5e9d51bbb..dc860f28e 100644 --- a/tool/src/test/resources/org/datacommons/tool/genmcf/statchecks/output/report.json +++ b/tool/src/test/resources/org/datacommons/tool/genmcf/statchecks/output/report.json @@ -222,7 +222,7 @@ "file": "covid.csv", "lineNumber": "31" }, - "userMessage": "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/06', variableMeasured: 'CumulativeCount_MedicalTest_ConditionCOVID_19_Positive', observationDate: '2020-02-03', value1: 3.0, value2: 8.0", + "userMessage": "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/06', variableMeasured: 'CumulativeCount_MedicalTest_ConditionCOVID_19_Positive', observationDate: '2020-02-03', value1: 3, value2: 8", "counterKey": "Sanity_InconsistentSvObsValues" }], "statsCheckSummary": [{ diff --git a/tool/src/test/resources/org/datacommons/tool/lint/statchecks/output/report.json b/tool/src/test/resources/org/datacommons/tool/lint/statchecks/output/report.json index 050f0b3fd..95f642994 100644 --- a/tool/src/test/resources/org/datacommons/tool/lint/statchecks/output/report.json +++ b/tool/src/test/resources/org/datacommons/tool/lint/statchecks/output/report.json @@ -21,110 +21,149 @@ } } }, - "entries": [{ - "level": "LEVEL_ERROR", - "location": { - "file": "covid.mcf", - "lineNumber": "17" + "entries": [ + { + "level": "LEVEL_ERROR", + "location": { + "file": "covid.mcf", + "lineNumber": "17" + }, + "userMessage": "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/06', variableMeasured: 'CumulativeCount_MedicalTest_ConditionCOVID_19_Positive', observationDate: '2020-02-03', value1: 8, value2: 3", + "counterKey": "Sanity_InconsistentSvObsValues" + } + ], + "statsCheckSummary": [ + { + "placeDcid": "geoId/07", + "statVarDcid": "CumulativeCount_MedicalTest_ConditionCOVID_19_Positive", + "measurementMethod": "", + "observationPeriod": "", + "scalingFactor": "", + "unit": "", + "validationCounters": [ + { + "counterKey": "StatsCheck_MaxPercentFluctuationGreaterThan100", + "problemPoints": [ + { + "date": "2020-03-02", + "values": [ + { + "value": { + "type": "NUMBER", + "value": "3" + }, + "locations": [ + { + "file": "covid.mcf", + "lineNumber": "49" + } + ] + } + ] + }, + { + "date": "2020-03-03", + "values": [ + { + "value": { + "type": "NUMBER", + "value": "7" + }, + "locations": [ + { + "file": "covid.mcf", + "lineNumber": "65" + } + ] + } + ] + } + ], + "percentDifference": 133.33 + } + ] + }, + { + "placeDcid": "geoId/06", + "statVarDcid": "CumulativeCount_MedicalTest_ConditionCOVID_19_Positive", + "measurementMethod": "", + "observationPeriod": "", + "scalingFactor": "", + "unit": "", + "validationCounters": [ + { + "counterKey": "StatsCheck_Inconsistent_Values", + "problemPoints": [ + { + "date": "2020-02-03", + "values": [ + { + "value": { + "type": "NUMBER", + "value": "8" + }, + "locations": [ + { + "file": "covid.mcf", + "lineNumber": "25" + } + ] + }, + { + "value": { + "type": "NUMBER", + "value": "3" + }, + "locations": [ + { + "file": "covid.mcf", + "lineNumber": "17" + } + ] + } + ] + } + ] + }, + { + "counterKey": "StatsCheck_Data_Holes", + "additionalDetails": "Possible data hole found. Dates in this series: 2020-01-30, 2020-02-02, 2020-02-03, 2020-03-02, 2020-03-03, 2020-05-03" + } + ] }, - "userMessage": "Found nodes with different values for the same StatVarObservation :: observationAbout: 'geoId/06', variableMeasured: 'CumulativeCount_MedicalTest_ConditionCOVID_19_Positive', observationDate: '2020-02-03', value1: 8.0, value2: 3.0", - "counterKey": "Sanity_InconsistentSvObsValues" - }], - "statsCheckSummary": [{ - "placeDcid": "geoId/07", - "statVarDcid": "CumulativeCount_MedicalTest_ConditionCOVID_19_Positive", - "measurementMethod": "", - "observationPeriod": "", - "scalingFactor": "", - "unit": "", - "validationCounters": [{ - "counterKey": "StatsCheck_MaxPercentFluctuationGreaterThan100", - "problemPoints": [{ - "date": "2020-03-02", - "values": [{ - "value": { - "type": "NUMBER", - "value": "3" - }, - "locations": [{ - "file": "covid.mcf", - "lineNumber": "49" - }] - }] - }, { - "date": "2020-03-03", - "values": [{ - "value": { - "type": "NUMBER", - "value": "7" - }, - "locations": [{ - "file": "covid.mcf", - "lineNumber": "65" - }] - }] - }], - "percentDifference": 133.33 - }] - }, { - "placeDcid": "geoId/06", - "statVarDcid": "CumulativeCount_MedicalTest_ConditionCOVID_19_Positive", - "measurementMethod": "", - "observationPeriod": "", - "scalingFactor": "", - "unit": "", - "validationCounters": [{ - "counterKey": "StatsCheck_Inconsistent_Values", - "problemPoints": [{ - "date": "2020-02-03", - "values": [{ - "value": { - "type": "NUMBER", - "value": "8" - }, - "locations": [{ - "file": "covid.mcf", - "lineNumber": "25" - }] - }, { - "value": { - "type": "NUMBER", - "value": "3" - }, - "locations": [{ - "file": "covid.mcf", - "lineNumber": "17" - }] - }] - }] - }, { - "counterKey": "StatsCheck_Data_Holes", - "additionalDetails": "Possible data hole found. Dates in this series: 2020-01-30, 2020-02-02, 2020-02-03, 2020-03-02, 2020-03-03, 2020-05-03" - }] - }, { - "placeDcid": "geoId/0601", - "statVarDcid": "CumulativeCount_MedicalTest_ConditionCOVID_19_Positive", - "measurementMethod": "", - "observationPeriod": "", - "scalingFactor": "", - "unit": "", - "validationCounters": [{ - "counterKey": "StatsCheck_Inconsistent_Date_Granularity", - "problemPoints": [{ - "date": "2020-03", - "values": [{ - "value": { - "type": "NUMBER", - "value": "1" - }, - "locations": [{ - "file": "covid.mcf", - "lineNumber": "105" - }] - }] - }] - }] - }], + { + "placeDcid": "geoId/0601", + "statVarDcid": "CumulativeCount_MedicalTest_ConditionCOVID_19_Positive", + "measurementMethod": "", + "observationPeriod": "", + "scalingFactor": "", + "unit": "", + "validationCounters": [ + { + "counterKey": "StatsCheck_Inconsistent_Date_Granularity", + "problemPoints": [ + { + "date": "2020-03", + "values": [ + { + "value": { + "type": "NUMBER", + "value": "1" + }, + "locations": [ + { + "file": "covid.mcf", + "lineNumber": "105" + } + ] + } + ] + } + ] + } + ] + } + ], "commandArgs": { "existenceChecks": true, "resolution": "RESOLUTION_MODE_LOCAL", From 1c61b079b40a41e5be28a7ad12b541ad03c400d4 Mon Sep 17 00:00:00 2001 From: Gabriel Mechali Date: Fri, 14 Aug 2026 15:09:36 -0400 Subject: [PATCH 4/5] Statcheck optimization --- .../org/datacommons/util/StatChecker.java | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/util/src/main/java/org/datacommons/util/StatChecker.java b/util/src/main/java/org/datacommons/util/StatChecker.java index 00c4bc9a4..b9ac4fb42 100644 --- a/util/src/main/java/org/datacommons/util/StatChecker.java +++ b/util/src/main/java/org/datacommons/util/StatChecker.java @@ -604,26 +604,28 @@ private boolean checkSvObsValueInconsistency(McfGraph.PropertyValues node) { } Long fp = hasher.hash().asLong(); String val = McfUtil.getPropVal(node, Vocabulary.VALUE); - if (this.svObValues.containsKey(fp) && !this.svObValues.get(fp).equals(val)) { - logCtx.addEntry( - Level.LEVEL_ERROR, - "Sanity_InconsistentSvObsValues", - "Found nodes with different values for the same StatVarObservation :: observationAbout: '" - + McfUtil.getPropVal(node, Vocabulary.OBSERVATION_ABOUT) - + "', variableMeasured: '" - + McfUtil.getPropVal(node, Vocabulary.VARIABLE_MEASURED) - + "', observationDate: '" - + McfUtil.getPropVal(node, Vocabulary.OBSERVATION_DATE) - + "', value1: " - + this.svObValues.get(fp) - + ", value2: " - + val, - node.getLocationsList()); - return false; - } else { - this.svObValues.put(fp, val); + String existingVal = this.svObValues.putIfAbsent(fp, val); + if (existingVal != null) { + if (!existingVal.equals(val)) { + logCtx.addEntry( + Level.LEVEL_ERROR, + "Sanity_InconsistentSvObsValues", + "Found nodes with different values for the same StatVarObservation :: observationAbout: '" + + McfUtil.getPropVal(node, Vocabulary.OBSERVATION_ABOUT) + + "', variableMeasured: '" + + McfUtil.getPropVal(node, Vocabulary.VARIABLE_MEASURED) + + "', observationDate: '" + + McfUtil.getPropVal(node, Vocabulary.OBSERVATION_DATE) + + "', value1: " + + existingVal + + ", value2: " + + val, + node.getLocationsList()); + return false; + } return true; } + return true; } public List getSamplePlaces() { From 1e7a5342f5a711a490a5c875603c254c7d83d475 Mon Sep 17 00:00:00 2001 From: Gabriel Mechali Date: Fri, 14 Aug 2026 15:14:09 -0400 Subject: [PATCH 5/5] StatChecker code cleanup of if branch --- .../org/datacommons/util/StatChecker.java | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/util/src/main/java/org/datacommons/util/StatChecker.java b/util/src/main/java/org/datacommons/util/StatChecker.java index b9ac4fb42..e3afa16be 100644 --- a/util/src/main/java/org/datacommons/util/StatChecker.java +++ b/util/src/main/java/org/datacommons/util/StatChecker.java @@ -605,25 +605,22 @@ private boolean checkSvObsValueInconsistency(McfGraph.PropertyValues node) { Long fp = hasher.hash().asLong(); String val = McfUtil.getPropVal(node, Vocabulary.VALUE); String existingVal = this.svObValues.putIfAbsent(fp, val); - if (existingVal != null) { - if (!existingVal.equals(val)) { - logCtx.addEntry( - Level.LEVEL_ERROR, - "Sanity_InconsistentSvObsValues", - "Found nodes with different values for the same StatVarObservation :: observationAbout: '" - + McfUtil.getPropVal(node, Vocabulary.OBSERVATION_ABOUT) - + "', variableMeasured: '" - + McfUtil.getPropVal(node, Vocabulary.VARIABLE_MEASURED) - + "', observationDate: '" - + McfUtil.getPropVal(node, Vocabulary.OBSERVATION_DATE) - + "', value1: " - + existingVal - + ", value2: " - + val, - node.getLocationsList()); - return false; - } - return true; + if (existingVal != null && !existingVal.equals(val)) { + logCtx.addEntry( + Level.LEVEL_ERROR, + "Sanity_InconsistentSvObsValues", + "Found nodes with different values for the same StatVarObservation :: observationAbout: '" + + McfUtil.getPropVal(node, Vocabulary.OBSERVATION_ABOUT) + + "', variableMeasured: '" + + McfUtil.getPropVal(node, Vocabulary.VARIABLE_MEASURED) + + "', observationDate: '" + + McfUtil.getPropVal(node, Vocabulary.OBSERVATION_DATE) + + "', value1: " + + existingVal + + ", value2: " + + val, + node.getLocationsList()); + return false; } return true; }