Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -495,13 +495,7 @@ 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()) {
value = obs.getText();
}

String value = obs.hasText() ? obs.getText() : "";
return Observation.builder().seriesKey(seriesKey).date(obs.getDate()).value(value).build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,21 +386,41 @@ 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 =
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();

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -114,23 +114,53 @@ 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()
.setSvObsSeries(
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);
PipelineResult.State state = p.run().waitUntilFinish();
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<McfGraph> input =
p.apply(
Create.of(
createStatVarObservationGraph(
"obsSigFig", "measurement_rate", "country/USA", "2020", "12.5000")));

PCollection<McfOptimizedGraph> 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
Expand Down
36 changes: 7 additions & 29 deletions util/src/main/java/org/datacommons/util/GraphUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,25 +191,7 @@ public static List<String> getPropertyValues(Map<String, McfGraph.Values> 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.
Expand Down Expand Up @@ -256,10 +238,10 @@ public static List<McfGraph> 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()) {
setPropVal(Property.value.name(), ValueType.NUMBER, Double.toString(o.getNumber()), node);
} else if (o.hasText()) {
setPropVal(Property.value.name(), ValueType.TEXT, o.getText(), node);
if (o.hasText()) {
String valText = o.getText();
ValueType valType = StringUtil.isNumber(valText) ? ValueType.NUMBER : ValueType.TEXT;
setPropVal(Property.value.name(), valType, valText, node);
}

// Set optional PVs.
Expand Down Expand Up @@ -329,12 +311,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<String, McfGraph.Values> entry : node.getPvsMap().entrySet()) {
String prop = entry.getKey();
Expand Down
14 changes: 4 additions & 10 deletions util/src/main/java/org/datacommons/util/StatChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,9 @@ public class StatChecker {
private final ConcurrentMap<String, Set<String>> 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<Long, Float> svObValues;
// same StatVarObservation with inconsistent values. Stored as raw String to preserve
// significant figures (SigFigs) and avoid floating-point precision collisions.
private final ConcurrentMap<Long, String> svObValues;
private final String EMPTY_PROP_STRING = "EMPTY_PROP";
private StatVarState statVarState;
private ExistenceChecker existenceChecker;
Expand Down Expand Up @@ -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)) {
Comment thread
gmechali marked this conversation as resolved.
Outdated
logCtx.addEntry(
Level.LEVEL_ERROR,
Expand Down
11 changes: 5 additions & 6 deletions util/src/main/proto/Mcf.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Comment thread
gmechali marked this conversation as resolved.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider just naming this value
Totally understand if it's not worth a bunch of extra overhead though


// Any additional PVs associated with the StatVarObs.
optional McfGraph.PropertyValues pvs = 4;
Expand Down
37 changes: 34 additions & 3 deletions util/src/test/java/org/datacommons/util/GraphUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<McfOptimizedGraph> 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<McfGraph> 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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down