Repository: pgmpy/pgmpy

Issue:
state_name and state_no in reduce()
### Subject of the issue
The value argument in reduce() function for TabularCPD and DiscreteFactor. It looks like right now the argument should be (var_name, var_state_name), but previously it's (var_name, var_state_no). However, in pre_compute_reduce() of Sampling.py, this for loop
```python
for state_combination in itertools.product(
            *[range(self.cardinality[var]) for var in variable_evid]
        ):
            states = list(zip(variable_evid, state_combination))
            cached_values[state_combination] = variable_cpd.reduce(
                states, inplace=False
            ).values
```
seems to use (var_name, var_state_no) as argument. 

### Your environment
* newest dev branch
* 3.6.8
* Ubuntu 18.04.2 LTS

### Steps to reproduce
I have a BayesianModel that has variable_state_name as str (not the same with variable_state_no). If I call likelihood_weighted_sample() on the model, which will call pre_compute_reduce(), it gives me key error, because reduce() calls self.get_state_no() like self.get_state_no(var_name, 0)

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/pgmpy/pgmpy/pull/1235?src=pr&el=h1) Report
> Merging [#1235](https://codecov.io/gh/pgmpy/pgmpy/pull/1235?src=pr&el=desc) into [dev](https://codecov.io/gh/pgmpy/pgmpy/commit/88ad46b1643fb50ffedef366563a60f9f4beafab?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pgmpy/pgmpy/pull/1235/graphs/tree.svg?width=650&token=UaJMCdHaEF&height=150&src=pr)](https://codecov.io/gh/pgmpy/pgmpy/pull/1235?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##              dev    #1235      +/-   ##
==========================================
+ Coverage   92.03%   92.04%   +<.01%     
==========================================
  Files         135      135              
  Lines       13200    13207       +7     
==========================================
+ Hits        12149    12156       +7     
  Misses       1051     1051
```


| [Impacted Files](https://codecov.io/gh/pgmpy/pgmpy/pull/1235?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pgmpy/factors/discrete/DiscreteFactor.py](https://codecov.io/gh/pgmpy/pgmpy/pull/1235/diff?src=pr&el=tree#diff-cGdtcHkvZmFjdG9ycy9kaXNjcmV0ZS9EaXNjcmV0ZUZhY3Rvci5weQ==) | `97.95% <100%> (+0.06%)` | :arrow_up: |
| [...py/tests/test\_factors/test\_discrete/test\_Factor.py](https://codecov.io/gh/pgmpy/pgmpy/pull/1235/diff?src=pr&el=tree#diff-cGdtcHkvdGVzdHMvdGVzdF9mYWN0b3JzL3Rlc3RfZGlzY3JldGUvdGVzdF9GYWN0b3IucHk=) | `99.82% <100%> (ø)` | :arrow_up: |
| [pgmpy/models/BayesianModel.py](https://codecov.io/gh/pgmpy/pgmpy/pull/1235/diff?src=pr&el=tree#diff-cGdtcHkvbW9kZWxzL0JheWVzaWFuTW9kZWwucHk=) | `90.06% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pgmpy/pgmpy/pull/1235?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pgmpy/pgmpy/pull/1235?src=pr&el=footer). Last update [88ad46b...2b7cbad](https://codecov.io/gh/pgmpy/pgmpy/pull/1235?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pgmpy/factors/discrete/DiscreteFactor.py b/pgmpy/factors/discrete/DiscreteFactor.py
index 7a8cc62d..dab8fbc3 100644
--- a/pgmpy/factors/discrete/DiscreteFactor.py
+++ b/pgmpy/factors/discrete/DiscreteFactor.py
@@ -2,6 +2,7 @@ from __future__ import division
 
 from itertools import product
 from collections import namedtuple
+from warnings import warn
 
 import numpy as np
 
@@ -415,6 +416,7 @@ class DiscreteFactor(BaseFactor, StateNameMixin):
         >>> phi.values
         array([0., 1.])
         """
+        # Check if values is an array
         if isinstance(values, str):
             raise TypeError("values: Expected type list or array-like, got type str")
 
@@ -423,10 +425,23 @@ class DiscreteFactor(BaseFactor, StateNameMixin):
                 "values: Expected type list of tuples, get type {type}", type(values[0])
             )
 
+        # Check if all variables in values are in the factor
+        for var, _ in values:
+            if var not in self.variables:
+                raise ValueError(f"The variable: {var} is not in the factor")
+
         phi = self if inplace else self.copy()
-        values = [
-            (var, self.get_state_no(var, state_name)) for var, state_name in values
-        ]
+
+        # Convert the state names to state number. If state name not found treat them as
+        # state numbers.
+        try:
+            values = [
+                (var, self.get_state_no(var, state_name)) for var, state_name in values
+            ]
+        except KeyError:
+            warn(
+                "Found unknown state name. Trying to switch to using all state names as state numbers"
+            )
 
         var_index_to_del = []
         slice_ = [slice(None)] * len(self.variables)
diff --git a/pgmpy/models/BayesianModel.py b/pgmpy/models/BayesianModel.py
index 270a7174..6f176823 100644
--- a/pgmpy/models/BayesianModel.py
+++ b/pgmpy/models/BayesianModel.py
@@ -508,10 +508,7 @@ class BayesianModel(DAG):
         <TabularCPD representing P(B:2) at 0x7fb98a7d5588>,
         <TabularCPD representing P(C:2 | A:2, B:2) at 0x7fb98a7b1f98>]
         """
-        from pgmpy.estimators import (
-            MaximumLikelihoodEstimator,
-            BaseEstimator,
-        )
+        from pgmpy.estimators import MaximumLikelihoodEstimator, BaseEstimator
 
         if estimator is None:
             estimator = MaximumLikelihoodEstimator


Test patch:
diff --git a/pgmpy/tests/test_factors/test_discrete/test_Factor.py b/pgmpy/tests/test_factors/test_discrete/test_Factor.py
index 1ed1b0e3..2f85510b 100644
--- a/pgmpy/tests/test_factors/test_discrete/test_Factor.py
+++ b/pgmpy/tests/test_factors/test_discrete/test_Factor.py
@@ -447,12 +447,12 @@ class TestFactorMethods(unittest.TestCase):
         self.assertRaises(TypeError, self.phi1.reduce, ["x10"])
 
     def test_reduce_scopeerror(self):
-        self.assertRaises(KeyError, self.phi1.reduce, [("x4", 1)])
-        self.assertRaises(KeyError, self.phi5.reduce, [((("x1", 0.1), 0))])
+        self.assertRaises(ValueError, self.phi1.reduce, [("x4", 1)])
+        self.assertRaises(ValueError, self.phi5.reduce, [((("x1", 0.1), 0))])
 
     def test_reduce_sizeerror(self):
-        self.assertRaises(KeyError, self.phi1.reduce, [("x3", 5)])
-        self.assertRaises(KeyError, self.phi5.reduce, [(("x2", "x3"), 3)])
+        self.assertRaises(IndexError, self.phi1.reduce, [("x3", 5)])
+        self.assertRaises(IndexError, self.phi5.reduce, [(("x2", "x3"), 3)])
 
     def test_identity_factor(self):
         identity_factor = self.phi.identity_factor()