--allow-missing: skips stages even if deps have changed

[dberenbaum]

Bug Report

Description

--allow-missing is skipping stages unexpectedly (in cases where pulling the data and then running the pipeline will not skip the stages).

Reproduce

In https://github.com/dberenbaum/example-get-started-experiments/tree/split-0.2, I changed the param data_split.test_pct from 0.15 -> 0.2 and ran only the data_split stage.

$ git clone -b split-0.2 git@github.com:dberenbaum/example-get-started-experiments.git
$ cd example-get-started-experiments
$ dvc exp run --allow-missing --dry
Reproducing experiment 'faint-kine'
'data/pool_data.dvc' didn't change, skipping
Stage 'data_split' didn't change, skipping
Stage 'train' didn't change, skipping
Stage 'evaluate' didn't change, skipping
$ dvc pull
$ dvc exp run --dry
Reproducing experiment 'extra-colt'
'data/pool_data.dvc' didn't change, skipping
Stage 'data_split' didn't change, skipping
Running stage 'train':
> python src/train.py

Running stage 'evaluate':
> python src/evaluate.py

Expected

I would expect that the train and evaluate stages would be run with or without --allow-missing, and that --allow-missing would run the same stages as it would if I first pulled and then ran without --allow-missing.

Environment information

Output of dvc doctor:

$ dvc doctor
DVC version: 2.58.1 (pip)
-------------------------
Platform: Python 3.10.10 on macOS-13.3.1-arm64-arm-64bit
Subprojects:
        dvc_data = 0.51.0
        dvc_objects = 0.22.0
        dvc_render = 0.5.3
        dvc_task = 0.2.1
        scmrepo = 1.0.3
Supports:
        azure (adlfs = 2023.4.0, knack = 0.10.1, azure-identity = 1.12.0),
        gdrive (pydrive2 = 1.15.3),
        gs (gcsfs = 2022.11.0),
        hdfs (fsspec = 2022.11.0, pyarrow = 11.0.0),
        http (aiohttp = 3.7.4.post0, aiohttp-retry = 2.8.3),
        https (aiohttp = 3.7.4.post0, aiohttp-retry = 2.8.3),
        oss (ossfs = 2021.8.0),
        s3 (s3fs = 2022.11.0, boto3 = 1.24.59),
        ssh (sshfs = 2023.4.1),
        webdav (webdav4 = 0.9.8),
        webdavs (webdav4 = 0.9.8),
        webhdfs (fsspec = 2022.11.0)
Config:
        Global: /Users/dave/Library/Application Support/dvc
        System: /Library/Application Support/dvc
Cache types: reflink, hardlink, symlink
Cache directory: apfs on /dev/disk3s1s1
Caches: local
Remotes: https
Workspace directory: apfs on /dev/disk3s1s1
Repo: dvc, git
Repo.site_cache_dir: /Library/Caches/dvc/repo/2ede911c0d219722caad26f44649bc87

[dberenbaum]

@daavoo I think it's a borderline p0 since it blocks the intended --allow-missing workflow.

[daavoo]

@daavoo I think it's a borderline p0 since it blocks the intended --allow-missing workflow.

Will take a look today.

Perhaps I am misunderstanding but this is about --dry, right? I thought without --dry and combining with --pull was also a relevant (if not the main) intended use case

[dberenbaum]

It's not only about --dry. Here's an example without --dry:

# Clone branch where params have been changed but only data_split stage has been run
$ git clone -b split-0.2 git@github.com:dberenbaum/example-get-started-experiments.git
$ cd example-get-started-experiments
# Change remote to avoid run-cache error for http
$ dvc remote add -d --local storage s3://dvc-public/remote/get-started-pools
# I would expect this to run the train stage but it skips it
$ dvc exp run --allow-missing --pull
Reproducing experiment 'rathe-coho'
                                                                               'data/pool_data.dvc' didn't change, skipping
Stage 'data_split' didn't change, skipping
Stage 'train' didn't change, skipping
Stage 'evaluate' didn't change, skipping

Ran experiment(s): rathe-coho
Experiment results have been applied to your workspace.

To promote an experiment to a Git branch run:

        dvc exp branch <exp> <branch>
# Doing a manual pull, the train stage doesn't get skipped
$ dvc pull
$ dvc exp run
Reproducing experiment 'minor-lush'
'data/pool_data.dvc' didn't change, skipping
Stage 'data_split' didn't change, skipping
Running stage 'train':
> python src/train.py
...

[dberenbaum]

Here's the issue:

dvc/dvc/stage/__init__.py

Line 312 in 6ace5ed

if allow_missing and status[str(dep)] == "deleted":

data/train_data has status "deleted," but it's also been modified by the previous stage, so we can't assume we should skip everything that's deleted.

[daavoo]

data/train_data has status "deleted," but it's also been modified by the previous stage, so we can't assume we should skip everything that's deleted.

I think the dependency status should be reported as modified, based on the dvc.lock, and it should supersede the deleted:

[daavoo]

@dberenbaum Do you think the comment above makes sense, and if yes, should it be always the case for status, or should that only be the case if allow-missing is passed?

[dberenbaum]

Yup, it makes sense.

@dberenbaum Do you think the comment above makes sense, and if so, should be always the case for status, or should that only be the case if allow-missing is passed?

I think it would be nice to add an --allow-missing flag to status (in the cli also) and have it only reported as modified in that case (looking at https://dvc.org/doc/command-reference/status#local-workspace-status, the default behavior is clearly all about checking what's actually in the workspace, and it's probably best to keep this logical consistency).