Build options validation

[pmachapman]

Part of sillsdev/serval#966
Fixes sillsdev/serval#306

---

This change is 

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.93%. Comparing base (8374e7b) to head (5d70d98).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
- Coverage   92.08%   91.93%   -0.15%     
==========================================
  Files         382      387       +5     
  Lines       24170    24368     +198     
==========================================
+ Hits        22256    22403     +147     
- Misses       1914     1965      +51     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ddaspit]

@ddaspit reviewed 9 files and all commit messages, and made 7 comments.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on Enkidu93 and pmachapman).

---

machine/jobs/nmt_build_options.py line 24 at r1 (raw file):

    fp16: bool | None = None
    tf32: bool | None = None
    save_strategy: str | None = None

Remove save_strategy.

---

machine/jobs/nmt_build_options.py line 34 at r1 (raw file):

    )

    device: int | None = None

Remove device.

---

machine/jobs/nmt_build_options.py line 35 at r1 (raw file):

    device: int | None = None
    num_beams: int | None = 2

The default should be None.

---

machine/jobs/build_clearml_helper.py line 125 at r1 (raw file):

def update_settings(settings: Settings, args: dict, task: Optional[Task], logger: logging.Logger, model: type[T]):

The T typevar isn't necessary. model: BaseModel should be sufficient.

---

machine/jobs/build_clearml_helper.py line 136 at r1 (raw file):

            raise TypeError(f"Build options could not be parsed: {e}") from e
        try:
            model.model_validate(build_options)

It would be good to add tests to check the validation.

---

machine/jobs/build_clearml_helper.py line 139 at r1 (raw file):

        except ValidationError as e:
            raise ValueError(f"Invalid build options: {e}") from e
        settings.update({settings.model_type: build_options})

This is a preexisting bug, but the build options are not getting merged in correctly for thot models. The model_type of thot should map to thot_align for alignment models and thot_mt for MT models.

---

pyproject.toml line 63 at r1 (raw file):

charset-normalizer = "^2.1.1"
urllib3 = "<2"
pydantic = "^2.13.4"

pydantic should be included in the jobs extra.

[pmachapman]

@pmachapman made 7 comments.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on ddaspit and Enkidu93).

---

pyproject.toml line 63 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

pydantic should be included in the jobs extra.

Done.

---

machine/jobs/build_clearml_helper.py line 125 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

The T typevar isn't necessary. model: BaseModel should be sufficient.

Done.

---

machine/jobs/build_clearml_helper.py line 136 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

It would be good to add tests to check the validation.

Done.

---

machine/jobs/build_clearml_helper.py line 139 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This is a preexisting bug, but the build options are not getting merged in correctly for thot models. The model_type of thot should map to thot_align for alignment models and thot_mt for MT models.

Done. I think I understand what you mean - please see the tests test_update_settings_valid_smt_build_options and test_update_settings_valid_word_alignment_build_options for examples of the input/output, and please let me know if I correctly understood the requirements.

The fix is the lines just below:

        if "thot_align" in build_options:
            settings.update({"thot_align": build_options["thot_align"]})
        if "thot_mt" in build_options:
            settings.update({"thot_mt": build_options["thot_mt"]})

---

machine/jobs/nmt_build_options.py line 24 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Remove save_strategy.

Done.

---

machine/jobs/nmt_build_options.py line 34 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Remove device.

Done.

---

machine/jobs/nmt_build_options.py line 35 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

The default should be None.

Done.