Repository: alchemistry/alchemlyb

Issue:
NAMD - decorrelation and equilibration detection fails in v0.7 and 1.0
Recent updates (to preprocessing.py, I believe) have broken our scripts' ability to use decorrelation and equilibration detection for NAMD output. I have attached an MWE that includes some sample data from alchemtest. The most recent version that works for these test cases is Alchemlyb 0.6.0. I have included sample logs for runs using 0.6, 0.7, and 1.0

Finally, it looks like decorrelation should work on the entire dataframe based on the documentation, but we have had to separate the dataframe by fep-lambda. Otherwise, decorrelate_u_nk returns just fep-lambda=0.

Any clarification or advice would be helpful!

MWE: [alchemlyb_crash_MWE.zip](https://github.com/alchemistry/alchemlyb/files/10024970/alchemlyb_crash_MWE.zip)


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/alchemistry/alchemlyb/pull/275?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry) Report
> Merging [#275](https://codecov.io/gh/alchemistry/alchemlyb/pull/275?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry) (02afb7f) into [master](https://codecov.io/gh/alchemistry/alchemlyb/commit/0916cba61d141e4da171c6891edb9f65aad51ac1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry) (0916cba) will **decrease** coverage by `0.11%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##           master     #275      +/-   ##
==========================================
- Coverage   98.69%   98.58%   -0.12%     
==========================================
  Files          26       26              
  Lines        1762     1761       -1     
  Branches      379      356      -23     
==========================================
- Hits         1739     1736       -3     
  Misses          3        3              
- Partials       20       22       +2     
```


| [Impacted Files](https://codecov.io/gh/alchemistry/alchemlyb/pull/275?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry) | Coverage Δ | |
|---|---|---|
| [src/alchemlyb/preprocessing/subsampling.py](https://codecov.io/gh/alchemistry/alchemlyb/pull/275/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry#diff-c3JjL2FsY2hlbWx5Yi9wcmVwcm9jZXNzaW5nL3N1YnNhbXBsaW5nLnB5) | `100.00% <ø> (ø)` | |
| [src/alchemlyb/parsing/gomc.py](https://codecov.io/gh/alchemistry/alchemlyb/pull/275/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry#diff-c3JjL2FsY2hlbWx5Yi9wYXJzaW5nL2dvbWMucHk=) | `92.38% <0.00%> (-0.96%)` | :arrow_down: |
| [src/alchemlyb/parsing/gmx.py](https://codecov.io/gh/alchemistry/alchemlyb/pull/275/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry#diff-c3JjL2FsY2hlbWx5Yi9wYXJzaW5nL2dteC5weQ==) | `97.60% <0.00%> (-0.60%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alchemistry)

xiki-tempula: @orbeckst I don't know why RTD failed but it works on my local computer

Patch:
diff --git a/CHANGES b/CHANGES
index 4bd7e8c..2ab16ab 100644
--- a/CHANGES
+++ b/CHANGES
@@ -22,6 +22,7 @@ Fixes
     is not found (issue #272, PR #273).
   - The regex in the AMBER parser now reads also 'field=value' pairs where
     there are no spaces around the equal sign (issue #272, PR #273).
+  - Pre-processing function slicing will not drop NaN rows (issue #274, PR #275).
 
 
 10/31/2022 orbeckst, xiki-tempula, DrDomenicoMarson 
diff --git a/src/alchemlyb/preprocessing/subsampling.py b/src/alchemlyb/preprocessing/subsampling.py
index 2b34274..3b8bd8a 100644
--- a/src/alchemlyb/preprocessing/subsampling.py
+++ b/src/alchemlyb/preprocessing/subsampling.py
@@ -380,6 +380,9 @@ def slicing(df, lower=None, upper=None, step=None, force=False):
     DataFrame
         `df` subsampled.
 
+
+    .. versionchanged:: 1.0.1
+       The rows with NaN values are not dropped by default.
     """
     try:
         df = df.loc[lower:upper:step]
@@ -391,9 +394,6 @@ def slicing(df, lower=None, upper=None, step=None, force=False):
                        "to use slicing on DataFrames with unique time values "
                        "for each row. Use `force=True` to ignore this error.")
 
-    # drop any rows that have missing values
-    df = df.dropna()
-
     return df
 
 


Test patch:
diff --git a/src/alchemlyb/tests/test_preprocessing.py b/src/alchemlyb/tests/test_preprocessing.py
index b0d7a60..120ca43 100644
--- a/src/alchemlyb/tests/test_preprocessing.py
+++ b/src/alchemlyb/tests/test_preprocessing.py
@@ -1,21 +1,21 @@
 """Tests for preprocessing functions.
 
 """
-import pytest
-
+import alchemtest.gmx
 import numpy as np
+import pytest
+from alchemtest.gmx import load_benzene
+from alchemtest.namd import load_idws
 from numpy.testing import assert_allclose
 
 import alchemlyb
-from alchemlyb.parsing import gmx
+from alchemlyb.parsing import gmx, namd
+from alchemlyb.parsing.gmx import extract_u_nk, extract_dHdl
 from alchemlyb.preprocessing import (slicing, statistical_inefficiency,
                                      equilibrium_detection,
                                      decorrelate_u_nk, decorrelate_dhdl,
                                      u_nk2series, dhdl2series)
-from alchemlyb.parsing.gmx import extract_u_nk, extract_dHdl
-from alchemtest.gmx import load_benzene, load_ABFE
 
-import alchemtest.gmx
 
 def gmx_benzene_dHdl():
     dataset = alchemtest.gmx.load_benzene()
@@ -86,6 +86,19 @@ class TestSlicing:
     def test_basic_slicing(self, data, size):
         assert len(self.slicer(data, lower=1000, upper=34000, step=5)) == size
 
+    def test_unchanged(self):
+        # NAMD energy files only have dE for adjacent lambdas, this ensures
+        # that the slicer will not drop these rows as they have NaN values.
+        file = load_idws().data['forward'][0]
+        u_nk = namd.extract_u_nk(file, 298)
+
+        # Do the pre-processing as the u_nk are from all lambdas
+        groups = u_nk.groupby('fep-lambda')
+        for key, group in groups:
+            group = group[~group.index.duplicated(keep='first')]
+            df = self.slicer(group, None, None, None)
+            assert len(df) == len(group)
+
     @pytest.mark.parametrize(('dataloader', 'lower', 'upper'),
                              [
                                  ('gmx_benzene_dHdl_fixture', 1000, 34000),