exp run: validate params filename and parameter names

[pmrowla]

One thing I noticed when trying the changes from #5302 is that if the user specifies a parameter or params file that doesn't exist (i.e. typo's a param name) we just silently accept it and create the new file or new entry in the existing params file (this behavior existed before the -S/--set-param change as well).

If the specified parameter doesn't exist in the target stage/pipeline we should probably throw an error instead of silently ignoring it.

Reproduce

git clone https://github.com/iterative/example-get-started
cd example-get-started
dvc pull
virtualenv .venv
. .venv/bin/activate
pip install -r src/requirements.txt
dvc exp run -S feature.n_grams=3
git diff params.yaml

The experiment is run as normal and params.yaml has a new parameter:

diff --git a/params.yaml b/params.yaml
index e5a636f..aed2663 100644
--- a/params.yaml
+++ b/params.yaml
@@ -6,6 +6,7 @@ featurize:
   max_features: 3000
   ngrams: 2
 
+  n_grams: 3
 train:
   seed: 20170428
   n_est: 100
(END)

Note the typo in the parameter name.

The user thinks that they have changed a parameter, and no error is reported.

Expected

I'd expect parameters' existence are checked before making any modifications. So the line with

dvc exp run -S train.n_grams=3

should report that train.n_grams is not a valid parameter.

Environment information

Output of dvc doctor:

$ dvc doctor
DVC version: 2.4.3 (pip)
---------------------------------
Platform: Python 3.8.10 on Linux-5.8.0-59-generic-x86_64-with-glibc2.29
Supports: All remotes
Cache types: hardlink, symlink
Cache directory: ext4 on /dev/nvme0n1p2
Caches: local
Remotes: https
Workspace directory: ext4 on /dev/nvme0n1p2
Repo: dvc, git

[dberenbaum]

Need to consider what's expected here. It could be useful for new users doing experiments who have no prior dvc experience to setup experiments without ever touching params.yaml by adding parameters this way. Maybe a warning is best?

[mattlbeck]

I have started to use dvc exp run in earnest recently. I have found this behaviour particularly unexpected and had assumed that misspelling a parameter would result in the experiment not running.

In my opinion the current behaviour is pretty dangerous and a major source of experiment error if the user does not validate that the params were correctly applied themselves afterwards.

I appreciate allowing users to stay away from params files, but I think some --verify-params flag would be very much appreciated at least.

[mmeendez8]

I understand @mattlbeck and @pmrowla point here.

I would like to add another view point here. When dealing with DL frameworks, most time you have model config and training parameters already defined in the framework configuration, see for example:

• mmdetection config files
• detectron config files

So if all parameters have to exist in params.yaml, I am forced to duplicate all my config parameters twice!

[dberenbaum]

@mattlbeck Do you think a warning would be sufficient, or do you think it's important for the experiment to fail?

@mmeendez8 You only need to add to params.yaml those parameters that you change via experiments, right? So basically validating the parameters means you have to edit params.yaml directly instead of using --set-param if you are introducing a new parameter?

[mattlbeck]

If it is important to maintain a route to modifying parameters solely via the CLI, then I would suggest a new --verify-params flag that fails the experiment when new params are declared via a dvc exp run.

This is in keeping with the idea that more experienced users are more likely to maintain parameters via the config file, as they will also be the same users who discover additional flags to use such as this one.

I would not suggest a warn by default, as that may contradict the kinds of best practice you are attempting to encourage.

[mmeendez8]

@dberenbaum yes, you are right. Problem here is when you are using the --queue flag. Would this work avoiding the flag, simply changing params.yaml and running multiple dvc exp run --queue?