RagnarGrootKoerkamp / BAPCtools

Tools for developing ICPC-style programming contest problems.
GNU General Public License v3.0
50 stars 23 forks source link

Many `test/problems/*/problem.yaml`s break the specification #290

Closed thorehusfeldt closed 8 months ago

thorehusfeldt commented 1 year ago

Background

In a problem.yaml, the key name specified as

Key Type Default Comments
name String or map of strings   Required. If a string this is the name of the problem in English. If a map the keys are language codes and the values are the name of the problem in that language. It is an error for a language to be missing if there exists a problem statement for that language.

should look like this:

name: Muffin

or this:

name:
  'en': Muffin
  'nl' : Stroopwafel
  'da': Wienerbrød

but not like this:

name:

or be missing.

The contents of a problem metadata configuration file problem.yaml are specified at

My best bet at expressing the specification in CUE is at

Observation

Various .yaml files in BAPCtools (typically intest/problems/*/problem.yaml, break a narrow reading of the the specification, typically by specifying key: null or missing the required name field.

(In fact, both of the yaml files provided as examples in the problem package specification at https://www.kattis.com/problem-package-format/examples/problem_yaml break the specification as well, but that’s not a BAPCtools issue.)

What to do

  1. Agree on a valid CUE specification of the problem metadata file, possibly by taking the above draft as a starting point. This requires discussion of what constructions like name: null should even mean, and what kind of legacy traditions exist.
  2. run cue vet ../../test/problems/fltcmp/problem.yaml *.cue -d "#problem_settings" etc. on all the problem.yaml files in BAPC-tools; edit as needed.
  3. consider doing something similar for contest-level yaml (problems.yaml, contest.yaml), this task includes specifying them in CUE
  4. automatically run cue vet to on problem.yaml files, as well as generators.yaml against their respective schemas to make sure that files on the master branch agree with their schemas (and the documentation).
RagnarGrootKoerkamp commented 1 year ago

Hmmm... I vaguely remember a discussion about this at some point, and I think kattis actually doesn't use this field and/or ignores it. The main reason BAPCtools uses it (instead of simply hardcoding the problem name in problem.xy.tex) is that this way it can be reused in solution.xy.tex without having to duplicate it.

So the name: null is probably only there to make BAPCtools sufficiently happy when parsing the problem.yaml, and it would only be actually needed with bt pdf or bt solutions, which for most test problems is probably never actually executed.

Automatically running cue vet in CI on all relevant yaml files in the repo would be cool.

thorehusfeldt commented 1 year ago

Automatically running cue vet in CI on all relevant yaml files in the repo would be cool.

Right? Imagine if the spec did that (so the example files would satisfy the specification they exemplify!)

BAPCtools itself might also do this as part of bt validate, provided it can find cue in its environment (so that BAPCtools remains usable also for people who don’t want cue around.) Should require very few lines of extra code. There is a python CUE wrapper called pycue, but I don’t see the value over just calling cue from python.

thorehusfeldt commented 1 year ago

Here’s my output:

> for i in ../../test/problems/*/problem.yaml; do echo $i; cue vet $i *.cue -d "#problem_settings"; done
../../test/problems/boolfind/problem.yaml
source: conflicting values null and string (mismatched types null and string):
    -
    ./problem.cue:9:14
../../test/problems/different/problem.yaml
name: incomplete value string | {}
../../test/problems/fltcmp/problem.yaml
source: conflicting values null and string (mismatched types null and string):
    -
    ./problem.cue:9:14
../../test/problems/guess/problem.yaml
name: incomplete value string | {}
../../test/problems/guessnoeofcheck/problem.yaml
name: incomplete value string | {}
../../test/problems/hello/problem.yaml
source: conflicting values null and string (mismatched types null and string):
    -
    ./problem.cue:9:14
../../test/problems/helloproblemtools/problem.yaml
name: incomplete value string | {}
../../test/problems/identity/problem.yaml
source: conflicting values null and string (mismatched types null and string):
    -
    ./problem.cue:9:14
../../test/problems/test_problem_config/problem.yaml
cannot reference optional field: source:
    ./problem.cue:11:8

As you can see (CUE output is not exactly friendly), the problems are mainly name and source fields. These are of course easily fixed. But we also want this for generators.yaml in all problems and in particular in the docs/generators.yaml.

RagnarGrootKoerkamp commented 1 year ago

Yeah, since it's not so readable it probably makes sense either as part of bt validate (which is currently only for input/answer files), or a new bt cue or bt validate-yaml or something like that. BAPCtools (especially `bt generate) can just keep giving it's own errors.

RagnarGrootKoerkamp commented 8 months ago

These have been fixed meanwhile. CI is passing.