Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hydra composition and use of variables from composed params.yaml in stage breaks commands #8486

Closed
aschuh-hf opened this issue Oct 29, 2022 · 14 comments

Comments

@aschuh-hf
Copy link

aschuh-hf commented Oct 29, 2022

Bug Report

Description

I am using Hydra composition to allow me to split up a configuration into separate files, and compose them together as needed for a given experiment. Thanks for adding this feature!

However, I encountered problems with my experiment setup because I am using variables from the composed configuration in the deps and outs keys of stages. The variables of the composed params.yaml file define directory paths of different intermediate pipeline artifacts (i.e. prepared training input data). The reason these are variables is two-fold:

  • The paths depend on a dataset name variable to make it easy to switch between datasets.
  • The paths depend on a data.store prefix which may either be a relative local path (e.g., data) or a S3 URI prefix (e.g., s3://bucket/key/prefix/). For the latter case, when a S3 URI is used, I have setup cache.s3 in .dvc/config to store prepared training data as external output / dependency of the main train stage. The idea being that I want to be able to run initial experiments on a local GPU server, and at a later stage experiments in AWS EC2 (Ray Cluster).

Reproduce

  1. dvc config hydra.enabled True
  2. Create conf/config.yaml with key path: input.txt
  3. Create dvc.yaml with stages
stages:
  stage_1:
    cmd: echo "Example output path" > ${path}
    outs:
    - ${path}
  stage_2:
    cmd: cat ${path}
    deps:
    - ${path}
  1. Run dvc status dvc.yaml which displays error
ERROR: failed to parse 'stages.stage_1.cmd' in 'dvc.yaml': Could not find 'path'
  1. Try dvc exp run stage_2, which also shows the error:
ERROR: failed to parse 'stages.stage_2.cmd' in 'dvc.yaml': Could not find 'path'

CORRECTION: dvc exp run does work, I had failed to enable hydra in the DVC config when running this test. The issue with dvc status when params.yaml does not exist remains, though.

Expected

dvc status and dvc exp show produce expected output rather than a failure to parse the dvc.yaml stages.

Environment information

Output of dvc doctor:

$ dvc doctor
DVC version: 2.30.0 (rpm)
---------------------------------
Platform: Python 3.8.3 on Linux-3.10.0-1160.15.2.el7.x86_64-x86_64-with-glibc2.14
Subprojects:

Supports:
        azure (adlfs = None, knack = 0.10.0, azure-identity = None),
        gdrive (pydrive2 = 1.14.0),
        gs (gcsfs = None),
        hdfs (fsspec = None, pyarrow = 9.0.0),
        http (aiohttp = None, aiohttp-retry = 2.8.3),
        https (aiohttp = None, aiohttp-retry = 2.8.3),
        oss (ossfs = 2021.8.0),
        s3 (s3fs = None, boto3 = None),
        ssh (sshfs = 2022.6.0),
        webdav (webdav4 = 0.9.7),
        webdavs (webdav4 = 0.9.7),
        webhdfs (fsspec = None)
Cache types: hardlink, symlink
Cache directory: xfs on /dev/md124
Caches: local, s3
Remotes: s3, s3
Workspace directory: xfs on /dev/md124
Repo: dvc (subdir), git

Additional Information (if any):

I've installed DVC in a Docker image directly from YUM repository.

RUN wget https://dvc.org/rpm/dvc.repo -O /etc/yum.repos.d/dvc.repo \
    && rpm --import https://dvc.org/rpm/iterative.asc \
    && yum update -y \
    && yum install -y dvc-2.28.0-1 \
    && yum clean all \
    && rm -rf /var/cache/yum

(The mismatch in DVC version is because the above I used when building the Docker image, but later in the container upgraded DVC with yum to a more recent version)

In my actual project, I had produced the params.yaml with a first stage that I had used to just test the Hydra composition:

stages:
  print_params:
    cmd: cat params.yaml
    outs:
    - params.yaml:
        cache: false
        persist: true

After this, the params.yaml existed already and I had it added to git with git add params.yaml + git commit because the file was not automatically added by dvc exp run to .gitignore.

With the params.yaml file existing, the dvc status and dvc exp run commands worked in my actual project. I only encountered the issue with dvc exp show and also when trying to display the experiments in VS Code using the DVC extension. Only when putting the example above together I noticed that even the initial dvc exp run would not work with this dvc.yaml.

@aschuh-hf
Copy link
Author

Note that when I remove the ${path} from the cmd, but instead read the value by parsing the params.yaml file in a Python script (prepare_data.py for stage_1 and consume_data.py for stage_2) which has no arguments that need to be inferred from the composed params.yaml file, the error shifts to the use of the variables in the outs field:

> dvc status
ERROR: failed to parse 'stages.stage_1.outs' in 'dvc.yaml': Could not find 'path'

@aschuh-hf
Copy link
Author

After a successful dvc exp run, I deleted the composed params.yaml and ran dvc exp show:

> dvc exp show
 ─────────────────────────────────
  Experiment   Created   Executor
 ─────────────────────────────────
  workspace    !         !
  74829bd      !         !
 ─────────────────────────────────

@aschuh-hf
Copy link
Author

aschuh-hf commented Oct 29, 2022

Apart from that dvc status only works after dvc exp run which produced a params.yaml file, I start to believe the issue I am seeing has rather to do with failed previous experiment runs. In the example here, I replaced the cmd of both stages with calling Python scripts but made a small mistake (i.e., ruamel.yaml.YAML(typ="safe").load("params.yaml") instead of ruamel.yaml.YAML(typ="safe").load(pathlib.Path("params.yaml"))), which caused the first dvc exp run to fail. After the fixing the bug in the script, dvc exp show seems to work fine:

> dvc exp show
 ───────────────────────────────────────────────────────────────────────────────
  Experiment                Created    State   Executor   path        input.txt
 ───────────────────────────────────────────────────────────────────────────────
  workspace                 -          -       -          input.txt   bebd3d8
  231aa6f                   !          -       !          !           -
  └── d9fe890 [exp-c933a]   02:36 PM   !       -          input.txt   bebd3d8
 ───────────────────────────────────────────────────────────────────────────────

PS: I am just starting to use dvc exp. Have so far used dvc repro instead and may need to find the right way of working with the new experiments feature correctly. What is bothering are the ! in the dvc exp show output (same as in "Experiments" tab of VS Code extension) for Git commits where the params.yaml is missing. I also am not sure how to remove those rows from the output. dvc exp remove does not work as these aren't experiments apparently, but references to Git commits simply? Or is it because the experiments had failed?

@dberenbaum
Copy link
Collaborator

Hi @aschuh-hf! The ${path} variable you reference in dvc.yaml must be read from params.yaml, so it's expected that DVC will complain or show errors in the experiment table if params.yaml does not exist.

conf contains the structure of what can be written to params.yaml, including the defaults list in conf/config.yaml, but the actual parameters for any experiment are undefined until you use dvc exp run to generate the params.yaml file (or otherwise generate that file).

@aschuh-hf
Copy link
Author

Ok. I think the issue seems more that dvc exp show and the VS Code extension show entries in the experiments table which don't seem to be actual experiments, at least not successful ones. The entries show Git commit SHAs rather than experiment names (IDs) and I cannot remove them with dvc exp remove. Why do these extra rows show up in the experiments table and how can I get rid of them?

Screen Shot 2022-10-29 at 10 51 43 PM

I tried to move the .dvc/cache/runs folder, but this didn't

Also, even after the params.yaml file was generated, the "DVC Tracked" section n VS Code displays the aforementioned error still.

Screen Shot 2022-10-29 at 10 40 13 PM

Using DVC VS Code Extension v0.5.8 and DVC version 2.30.

@dberenbaum
Copy link
Collaborator

dberenbaum commented Nov 1, 2022

The entries show Git commit SHAs rather than experiment names (IDs) and I cannot remove them with dvc exp remove. Why do these extra rows show up in the experiments table and how can I get rid of them?

These are previous commits, and there is no way to delete commits from your history, so you can't remove them like you can with experiments. They are shown because users often find it helpful to compare experiments to historical results. I don't think VS Code has anyway to hide them for now, but please feel free to open a feature request in https://github.com/iterative/vscode-dvc/issues (or I can transfer the existing issue over there if you prefer). Are they causing a particular problem, or you would simply prefer to avoid the noise generated by them?

Also, even after the params.yaml file was generated, the "DVC Tracked" section n VS Code displays the aforementioned error still.

The VS Code extension needs to compare the current state to the last commit to determine DVC-tracked changes. Since the last commit does not have params.yaml, you are seeing an error. If you commit params.yaml, it should disappear.

Edit: cc @shcheklein

@shcheklein
Copy link
Member

@aschuh-hf yes, please create an issue in VS Code extension repo to make it more flexible (adjust which commits to show/analyze).

Should we also improve the message - explain that it's in the HEAD commit, not in the workspace?

On a side note- we should also do something with [31m and 39m] cc @skshetry - should we skip adding colors in --json mode? (it makes sense, since most likely this is expected to be consumed by a machine)

@aschuh-hf
Copy link
Author

aschuh-hf commented Nov 2, 2022

Thanks for clarifications. I'll open a separate issue for the VS Code extension.

Are they causing a particular problem, or you would simply prefer to avoid the noise generated by them?

These are not causing an issue, but having them shown red and with exclamation marks just gave the impression something ain't right which made me question either my use of dvc exp or the functionality thereof. Knowing why and how to interpret it helps. But then it would indeed still be nicer to not have them show up. If these are no DVC experiment runs, but just commits in my history, why would I want to compare with them?

If you commit params.yaml, it should disappear.

Ok, I'll do that. I actually had removed the params.yaml with git rm after the initial run of dvc exp where I added it because I was under the impression this should be a derived file similar to a build artifact. By having it removed and added to the .gitignore list, it is shown grayed out in VS Code, which reminds me not to edit this file because it is generated by dvc exp run. What would happen if I make local changes to params.yaml because I forgot that I should change the actual source of this "run" artifact in the conf/ folder instead? Will a dvc exp run warn me or just discard my changes, potentially loosing them? Or would dvc exp run "stash" those changes, then reapply them after the hydra composition of the files in conf/?

@skshetry
Copy link
Member

skshetry commented Nov 2, 2022

On a side note- we should also do something with [31m and 39m] cc @skshetry - should we skip adding colors in --json mode? (it makes sense, since most likely this is expected to be consumed by a machine)

It's not coming from --json, json already strips out color if it is not running in a tty. It's from our logger which uses colorama, somehow colorama tty detection is broken in dvc. Looking into it.

@dberenbaum
Copy link
Collaborator

But then it would indeed still be nicer to not have them show up. If these are no DVC experiment runs, but just commits in my history, why would I want to compare with them?

Yup, makes sense to be able to hide them. Thanks for the feedback!

What would happen if I make local changes to params.yaml because I forgot that I should change the actual source of this "run" artifact in the conf/ folder instead? Will a dvc exp run warn me or just discard my changes, potentially loosing them? Or would dvc exp run "stash" those changes, then reapply them after the hydra composition of the files in conf/?

dvc exp run will stash those changes but it will not reapply them unless you are queuing the experiment, in which case the changes should be preserved in your workspace. There's a discussion about this in iterative/vscode-dvc#1956 and a feature request in #7955 to make it easy to recover these changes, so I think we need to get around to making it possible/easy to recover the stashed changes.

@aschuh-hf
Copy link
Author

Thanks, that's good to know there is ongoing discussion on this. Feel free to close this issue.

@aschuh-hf
Copy link
Author

After adding the params.yaml back with git add and git commit, I can see now that the Git commits which are not associated with a particular DVC experiment are no longer red and the rows in the Experiments table show the entries from the params.yaml at these commits. Also the "DVC Tracked" section in the file explorer works.

In summary, the issue was caused by me removing the params.yaml from Git and not being clear that this file, which is generated by dvc exp run should indeed be tracked with Git.

@dberenbaum
Copy link
Collaborator

It's not coming from --json, json already strips out color if it is not running in a tty. It's from our logger which uses colorama, somehow colorama tty detection is broken in dvc. Looking into it.

@skshetry Should we keep the issue open to look into this, or do you want to track it somewhere else?

@daavoo
Copy link
Contributor

daavoo commented Feb 6, 2023

@skshetry Should we keep the issue open to look into this, or do you want to track it somewhere else?

We could move the discussion to #8957

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants