Repository: pgmpy/pgmpy

Issue:
Whitespaces in variable names behave as delimiters in XDSL
The following minimal working example throws an error during the file read.

```python
model = DiscreteBayesianNetwork()
model.add_nodes_from(["A B", "C"])
model.add_edges_from([("A B", "C")])

cpd_a = TabularCPD("A B", 2, [[0.2], [0.8]])
cpd_c = TabularCPD(
    "C", 2, [[0.1, 0.4], [0.9, 0.6]], evidence=["A B"], evidence_card=[2]
)

model.add_cpds(cpd_a, cpd_c)

writer = XDSLWriter(model)
writer.write_xdsl("issue_model.xdsl")

reader = XDSLReader("issue_model.xdsl")
model = reader.get_model()
```

I guess the whitespace is a delimiter in XDSL?

```
<cpt id="C">
    <state id="0"/>
    <state id="1"/>
    <parents>A B</parents>
    <probabilities>0.1000000000000000 0.9000000000000000 0.4000000000000000 0.6000000000000000</probabilities>
</cpt>
```

Right now I am just manually exchanging all whitespaces in variable names with underscores, but a warning and maybe an automatic exchange of whitespaces in variable names everywhere in the model would be nice, since variable names with whitespaces are allowed by pgmpy.

Discussion:
@ankurankan: @StefanNeu Thanks for reporting the issue. This happens because whitespace is the delimiter between variables in the parents tag, so the code currently doesn't distinguish between separate variable names vs whitespace in a variable name. A possible solution could be to do a variable name match when extracting the parents. @Nimish-4 Thoughts? @Nimish-4: @ankurankan The current implementation does simply consider the whitespace as the delimiter. A straightforward solution would be to just match whichever node is present in the parents' string. Maybe a user warning too incase whitespaces are detected in the parent string.

Solution patch:
diff --git a/pgmpy/readwrite/XDSL.py b/pgmpy/readwrite/XDSL.py
index 5315a3b72..b8f85d328 100644
--- a/pgmpy/readwrite/XDSL.py
+++ b/pgmpy/readwrite/XDSL.py
@@ -6,6 +6,7 @@
 import networkx as nx
 
 from pgmpy.factors.discrete import TabularCPD
+from pgmpy.global_vars import logger
 from pgmpy.models import DiscreteBayesianNetwork
 from pgmpy.utils import compat_fns
 
@@ -13,6 +14,8 @@
 class XDSLReader(object):
     """
     Initializes the reader object for XDSL file formats[1] created through GeNIe[2].
+    Note that XDSLReader only supports cpt blocks from the XDSL file format; elements like
+    'deterministic' need to be aapropriately converted into 'cpt' elements before usage.
 
     Parameters
     ----------
@@ -45,6 +48,7 @@ def __init__(self, path=None, string=None):
         else:
             raise ValueError("Must specify either path or string")
         self.network_name = self.network.attrib["id"]
+        self.cpt_elements = self.network.find("nodes").findall("cpt")
         self.variables = self.get_variables()
         self.variable_parents = self.get_parents()
         self.edge_list = self.get_edges()
@@ -61,8 +65,13 @@ def get_variables(self):
         >>> reader.get_variables()
         ['asia', 'tub', 'smoke', 'lung', 'either', 'xray', 'bronc', 'dysp']
         """
-        nodes = self.network.find("nodes")
-        variables = [variable.attrib["id"] for variable in nodes.findall("cpt")]
+        variables = [variable.attrib["id"] for variable in self.cpt_elements]
+        for var in variables:
+            if isinstance(var, str) and (" " in var):
+                raise ValueError(
+                    f"XDSLReader does not support models with node names that contain whitespaces. Failed to process node: {var}"
+                )
+
         return variables
 
     def get_parents(self):
@@ -84,8 +93,7 @@ def get_parents(self):
         }
         """
         variable_parents = {}
-        nodes = self.network.find("nodes").findall("cpt")
-        for node in nodes:
+        for node in self.cpt_elements:
             parents = node.find("parents")
             if parents is not None:
                 variable_parents[node.attrib["id"]] = parents.text.split(" ")
@@ -137,9 +145,8 @@ def get_states(self):
         dysp': ['Absent', 'Present']
         }
         """
-        nodes = self.network.find("nodes").findall("cpt")
         variable_states = {}
-        for cpt in nodes:
+        for cpt in self.cpt_elements:
             variable_states[cpt.attrib["id"]] = [
                 state.attrib["id"] for state in cpt.findall("state")
             ]
@@ -164,8 +171,7 @@ def get_values(self):
         }
         """
         variable_CPD = {}
-        nodes = self.network.find("nodes").findall("cpt")
-        for cpt in nodes:
+        for cpt in self.cpt_elements:
 
             combined_prob = cpt.find("probabilities")
             num_states = len([state for state in cpt.findall("state")])
@@ -314,6 +320,10 @@ def get_variables(self):
         nodes_elem = etree.SubElement(self.root, "nodes")
 
         for var in self.model.nodes:
+            if isinstance(var, str) and " " in var:
+                logger.warning(
+                    f" Node '{var}' contains whitespaces. This could cause issues, especially when using pgmpy.readwrite.XDSLReader"
+                )
             variable_tag[var] = etree.SubElement(nodes_elem, "cpt", {"id": var})
 
         return variable_tag


Test patch:
diff --git a/pgmpy/tests/test_readwrite/test_xdsl.py b/pgmpy/tests/test_readwrite/test_xdsl.py
index 85a705610..392986f19 100644
--- a/pgmpy/tests/test_readwrite/test_xdsl.py
+++ b/pgmpy/tests/test_readwrite/test_xdsl.py
@@ -1,5 +1,6 @@
 import os
 import unittest
+import warnings
 import xml.etree.ElementTree as etree
 
 import numpy as np
@@ -64,11 +65,33 @@
 	</nodes>
 </smile>"""
 
+TEST_WHITESPACE_MODEL = """<?xml version="1.0" encoding="UTF-8"?>
+<!-- This network was created in trial version of GeNIe, which can be used for evaluation purposes only -->
+<smile version="1.0" id="Asia" numsamples="10000" discsamples="10000">
+	<nodes>
+		<cpt id="node 1" diagtype="observation" ranked="true">
+			<state id="no" />
+			<state id="yes" />
+			<probabilities>0.5 0.5</probabilities>
+		</cpt>
+		<cpt id="node 2" diagtype="target">
+			<state id="no" label="F5" />
+			<state id="yes" label="F6" fault="true" />
+			<parents>node 1</parents>
+			<probabilities>0.5 0.5 0.5 0.5</probabilities>
+		</cpt>
+	</nodes>
+</smile>"""
+
 
 class TestXDSLReaderMethodsString(unittest.TestCase):
     def setUp(self):
         self.reader = XDSLReader(string=TEST_FILE)
 
+    def test_whitespace_error(self):
+        with self.assertRaises(ValueError):
+            self.model_with_whitespace = XDSLReader(string=TEST_WHITESPACE_MODEL)
+
     def test_get_variables(self):
         var_expected = [
             "asia",
@@ -209,6 +232,24 @@ def setUp(self):
         )  # testing without state names
         self.writer_dummy = XDSLWriter(self.dummy_model)
 
+        self.model_with_whitespaces = DiscreteBayesianNetwork()
+        self.model_with_whitespaces.add_nodes_from(["first node", "second node"])
+        self.model_with_whitespaces.add_edges_from([("first node", "second node")])
+        cpd_a = TabularCPD("first node", 2, [[0.5], [0.5]])
+        cpd_b = TabularCPD(
+            "second node",
+            2,
+            [[0.5, 0.5], [0.5, 0.5]],
+            evidence=["first node"],
+            evidence_card=[2],
+        )
+        self.model_with_whitespaces.add_cpds(cpd_a, cpd_b)
+
+    def test_whitespace_warning(self):
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            self.model_with_whitespaces_xdsl = XDSLWriter(self.model_with_whitespaces)
+
     def assert_models_equivalent(self, expected, got):
         self.assertSetEqual(set(expected.nodes()), set(got.nodes()))
         for node in expected.nodes():
@@ -246,6 +287,10 @@ def setUp(self):
         config.set_backend("torch")
         self.reader = XDSLReader(string=TEST_FILE)
 
+    def test_whitespace_error(self):
+        with self.assertRaises(ValueError):
+            self.model_with_whitespace = XDSLReader(string=TEST_WHITESPACE_MODEL)
+
     def test_get_variables(self):
         var_expected = [
             "asia",
@@ -359,6 +404,24 @@ def setUp(self):
         )  # testing without state names
         self.writer_dummy = XDSLWriter(self.dummy_model)
 
+        self.model_with_whitespaces = DiscreteBayesianNetwork()
+        self.model_with_whitespaces.add_nodes_from(["first node", "second node"])
+        self.model_with_whitespaces.add_edges_from([("first node", "second node")])
+        cpd_a = TabularCPD("first node", 2, [[0.5], [0.5]])
+        cpd_b = TabularCPD(
+            "second node",
+            2,
+            [[0.5, 0.5], [0.5, 0.5]],
+            evidence=["first node"],
+            evidence_card=[2],
+        )
+        self.model_with_whitespaces.add_cpds(cpd_a, cpd_b)
+
+    def test_whitespace_warning(self):
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            self.model_with_whitespaces_xdsl = XDSLWriter(self.model_with_whitespaces)
+
     def assert_models_equivalent(self, expected, got):
         self.assertSetEqual(set(expected.nodes()), set(got.nodes()))
         for node in expected.nodes():