diff --git a/README.md b/README.md index cd612b1..3fb6316 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ Everything else runs without them. Python 3.11 or newer. | --- | --- | --- | | `./gen_bittings.py` | `sfic-gen-bittings` | Random bittings from a parity pattern, or with any parity if you give none, optionally staying away from existing bittings, and following the [key shape rules](#key-shape-rules). | | `./check_bittings.py` | `sfic-check-bittings` | Quick check of a few `NAME=BITTING` values: format, parity, adjacent-cut limit, pairwise closeness, with advice on the [key shape rules](#key-shape-rules). | -| `./check_system.py` | `sfic-check-system` | Whole-scheme check of a system file: per-key rules, duplicates, closeness, operating-set sizes, cross-operation, residual risk. | +| `./check_system.py` | `sfic-check-system` | Whole-scheme check of a system file: per-key rules, duplicates, closeness, operating-set sizes, cross-operation, residual risk, and advice on the [key shape rules](#key-shape-rules). | | `./solve_system.py` | `sfic-solve-system` | Fills in the `null` bittings of a system file by random search plus hill climbing, then runs the full check. | | `./pin_system.py` | `sfic-pin-system` | Prints the pinning chart of every core of a system file that sets `pinning`, in the layout `check_charts` reads: the pins to put in each chamber, with `--draw` a drawing of the stacks and with `--pdf` one PDF to print. | | `./check_charts.py` | `sfic-check-charts` | Checks that the tools' pinning rules reproduce pinning charts from your keying software (`.txt` files kept outside this repository). Reports positions only, never key data. | @@ -121,7 +121,11 @@ The reasons, the numbers behind the defaults and what each costs in key space ar the `masters` of any core). The system file sets them in an optional `shape` object, with the rule names of the table above and `null` to turn one off (see [System file format](#system-file-format)). -- `check_system` does not use the rules yet. +- `check_system.py` prints the same advice for the keys and control keys of a system file, in + a "Key shape" section, using the file's `shape` settings. Decoded unit keys and retired keys + are left out (real keys already cut cannot be changed, and about half of any real set breaks + `max_run` = 1 without a pattern), and, as in `check_bittings`, it does not count as a problem + or change the exit status. ```bash ./gen_bittings.py --pins 7 -n 3 --master # three masters, no parity pattern diff --git a/TODO.md b/TODO.md index 4fb5b9e..779eecf 100644 --- a/TODO.md +++ b/TODO.md @@ -2,6 +2,14 @@ ## Ideas for later +- The solver's objective still rewards change keys that hold a master's cut, and keys far from + their master, because both shrink the count of valid false keys: after the shape rules a + change key shares a cut with a master at 26.8% of positions against 20% by chance + (docs/designs/key-shape-rules.md, "What building it showed"). Changing what is scored needs + its own measurement and decision (D6). +- Exact reachability bounds for the shape rules (the parity pattern and how the rules combine), + so that a rule set no key can meet is refused when the file is read, not when the search + gives up (the bounds look at each rule alone). - A solver for pinnable answers among more than three unknown keys that share cores (a system with every key blank): joint moves, or splitting the chain of cores (docs/designs/pinnable-solving.md, "Building pinnable keys exactly"). diff --git a/docs/decisions.md b/docs/decisions.md index 3b6282b..c597cad 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -705,3 +705,17 @@ choose and never silently relax; the exact counts and the risk figures are uncha without `shape`, which is deliberate (D6). The design is in its own document and has been accepted. Detail: [designs/key-shape-rules.md](designs/key-shape-rules.md). + +## D65. A key with no bitting that follows the shape rules is left null + +When the solver cannot find a bitting for a key that follows the file's shape rules (a tight +rule set, or a thin pinnable set), it does not relax a rule. It leaves the key `null` in the +written file, prints a `NOT SOLVED` line that says which rules turned the draws down (before +`Wrote` and again last, as in D59), and skips the full check, which would reject the null. +Keys built together name the key that broke the rules in most draws, and the others say they +waited for it. The file's `shape` object, or a known key, is the way out. `check_system` and +`check_bittings` only advise about existing keys, in `SHAPE` lines that change neither the +exit status nor the counts. + +Detail: [design.md, "The code"](design.md#the-code) and +[designs/key-shape-rules.md](designs/key-shape-rules.md). diff --git a/docs/design.md b/docs/design.md index 1bc4674..b6979cb 100644 --- a/docs/design.md +++ b/docs/design.md @@ -159,6 +159,17 @@ by a separate bounded search for each sample. For keys built together, the `NOT reason names a key only if it broke a rule in at least 80% of the draws (any random key breaks one now and then); the others say they were built together with it. +**Shape advice in the checker** (D64). `check_system` and `check_bittings` print the keys that +break a rule as `SHAPE` lines, with pin positions and never cuts, in a "Key shape" section that +is left out when every rule is off. They are advice: they change neither the exit status nor +the count of problems or warnings, and decoded unit keys and retired keys are not checked. +Retired keys are history, which is also why D11 exempts them from parity and the adjacent-cut +limit. Decoded unit keys are exempt from the `SHAPE` lines only (parity and the adjacent-cut +limit still apply to them): the vendor cut them, so there is nothing to change, and without a +parity pattern about half of any real set breaks `max_run` = 1, so anything louder would be +noise. The rules never enter the exact counts or the +residual-risk figures: they filter what the tools choose and leave what the tools count alone. + ## The key space The rules that decide which bittings can be cut live in one frozen object, diff --git a/docs/designs/key-shape-rules.md b/docs/designs/key-shape-rules.md index f4b61d9..aca86a6 100644 --- a/docs/designs/key-shape-rules.md +++ b/docs/designs/key-shape-rules.md @@ -1,11 +1,13 @@ # Rules for the shape of a generated key -Status: Accepted - -This document proposes a small set of rules about what a key's cuts look like (not -which keys can be pinned together) and a way to apply them to the keys the tools -generate: the solver's unknown keys and `gen_bittings`' output. It is written before -any code, and accepted by the maintainer ([D64 in decisions.md](../decisions.md)). The numbers are measured on fake +Status: Implemented + +This document describes a small set of rules about what a key's cuts look like (not +which keys can be pinned together) and the way they are applied to the keys the tools +generate: the solver's unknown keys and `gen_bittings`' output. It was written before +any code, accepted by the maintainer and built in the five steps of its plan +([D64 and D65 in decisions.md](../decisions.md)); "What building it showed" says what the +build added. The numbers are measured on fake systems by throwaway scripts that are not in the repository, and are illustrations of scale, not claims about any real system. @@ -128,10 +130,11 @@ only the 7% of bittings with three in a row and leaves the pairs, which is the c NOT SOLVED line names the key and the rules, before `Wrote ...` and again last. - **Keys that already exist** are not changed. `check_system` and `check_bittings` print the ones that break a rule as an advisory `SHAPE` line, with positions only and never - values. It does not change the exit status or the count of warnings. Decoded unit - keys and retired keys are left out (the vendor cut the first, and the second are history, - as for parity and MACS in D11), and without a pattern about half of any real set breaks - `max_run` = 1, so anything louder would be noise. + values. It does not change the exit status or the count of warnings. Retired keys are + left out because they are history, which is also why D11 exempts them from parity and MACS. + Decoded unit keys are left out of the `SHAPE` lines only (parity and MACS still apply to + them): the vendor cut them, so there is nothing to change, and without a pattern about half + of any real set breaks `max_run` = 1, so anything louder would be noise. ## Why not part of KeySpace @@ -206,12 +209,16 @@ Separate commits, each with tests: a range test (not exact figures) on the share of keys with equal neighbours, run with and without a pattern; and the "no candidate" report. 5. The advisory lines in `check_system`; README, `docs/design.md` and the file-format notes - in the same commit as the behaviour they describe. + in the same commit as the behaviour they describe. (The README's `shape` row and the + solver's paragraph came with step 4, and the generator's flags with step 3, as the + behaviour did.) A refactor, if one is needed to share the check between the tools, is its own commit and shows seeded output unchanged for files that turn every rule off. -## What cannot be known yet +## What building it showed + +These were open questions when the design was accepted. - Whether the rules can be met together. The reachability bounds look at each rule alone and ignore the parity pattern, so a combination can pass them and still have no key: 3 pins @@ -219,10 +226,19 @@ shows seeded output unchanged for files that turn every rule off. cuts has unless it is monotone, which the default rules forbid. The tool then runs to its attempt limit and says so, naming the rule that turned down most of the draws, and does not relax anything. Three pins is not a real key, so the bounds were left simple. -- How many pinnable candidates survive the rules in a system with many known keys: a unit - master that must also take a decoded unit's neighbours has a thin pinnable set already, - and the first run on such a file will say whether `master_min_span` = 6 is too strict. -- Whether single-position moves still connect the valid keys once `max_run` = 1 applies - without a pattern; I expect so (the share kept is 46%) and the search tests will show. -- Whether holding and spreading, the objective's bias, still produces odd keys once the - rules are in. That is the measurement to make after step 4, with the same scripts. +- **A thin pinnable set can leave no key that follows the rules.** A unit master among seven + random decoded units (a fake system) had 24 pinnable masters, and all 24 ended in two equal + cuts, so with the defaults the solver reports `NO pinnable bitting follows the shape rules` + and leaves the key `null`. It is reported, not repaired: the way out is the `shape` object + (for example `max_run: null`) or a known key. How often a real system hits this depends on + how many unit keys are decoded, and cannot be known from fake systems. A key built together + with the one that has none is left null too, and says whose rules it waited for (D65). +- **Single-position moves still connect the valid keys.** The hill climb and the draws never + got stuck in the 100 seeded solves measured (60 with a parity pattern, 40 with pinning and + none) or in the tests; no solve ended in `NOT SOLVED` for want of a shaped key. +- **Holding and spreading, the objective's bias, is still there.** After the rules, a change + key shares a cut with a master at 26.8% of positions (parity pattern, 60 seeds), against + 24.7% before and 20% by chance, so the solver still likes to hold cuts. The rules removed + the shapes it produced (equal neighbours 64% to 0%, a master's mean cut about 1 to 4.2, + total variation 12.1 to 14.8 against 15.0 for a uniform draw) and did not touch the cause. + Changing what is scored needs its own measurement and decision (D6); it is in TODO.md. diff --git a/sfic_solver/check_system.py b/sfic_solver/check_system.py index f26e721..730e968 100644 --- a/sfic_solver/check_system.py +++ b/sfic_solver/check_system.py @@ -4,6 +4,9 @@ Reads a JSON description of the system and reports: * per-key problems (parity and adjacent-cut limit for keys and control keys; duplicates across all keys) + * advice on the shape of each key (the file's shape rules: equal adjacent cuts, a depth used + too often, a key that only goes one way, a master that is too narrow); positions only, not + counted as problems, and decoded unit keys and retired keys are left out * pairs of keys that are too close * for each core type, how many keys operate it (intended + false keys) * known keys that would operate a core they are NOT meant to operate @@ -25,6 +28,9 @@ the pin count if smaller); pairs involving unit keys are skipped unless close_check_units is true (cross-operation is the real test) + shape optional object of key shape rules (max_run, max_same_depth, + forbid_monotone, master_min_span, min_total_variation; null turns one + off); see the README unit_prefix key-name prefix for unit keys (default "unit:") unit_count total number of units, to estimate undecoded-key risk keys {name: bitting} operating keys, including decoded unit keys @@ -144,6 +150,16 @@ def main(argv=None): problems += 1 print("done") + if not cfg.shape.is_off(): + print("\n== Key shape: advice only (decoded unit keys and retired keys are not " + "checked) ==") + masters = cfg.master_keys + checked = {**{n: c for n, c in keys.items() if not n.startswith(unit_prefix)}, **control} + advice = [f"SHAPE {name}: {cfg.shape.describe(rule, pins)}" + for name, cuts in checked.items() + for rule, pins in cfg.shape.violations(cuts, name in masters)] + print("\n".join(capped(advice) or ["none"])) + print(f"\n== Closeness (non-unit keys differing in fewer than {min_diff} positions) ==") scope = {n: c for n, c in everything.items() if cfg.close_check_units or not n.startswith(unit_prefix)} diff --git a/sfic_solver/config.py b/sfic_solver/config.py index cd97cae..f1ab00e 100644 --- a/sfic_solver/config.py +++ b/sfic_solver/config.py @@ -45,6 +45,11 @@ class Config: # covers_units} shape: model.ShapeRules = field(default_factory=model.ShapeRules) # the file's `shape` + @property + def master_keys(self): + """The names of the keys that are masters: those in the `masters` of any core.""" + return {m for core in self.cores for m in core["masters"]} + def _no_duplicate_keys(pairs): seen = {} diff --git a/sfic_solver/model.py b/sfic_solver/model.py index 6b9b089..d9d15c4 100644 --- a/sfic_solver/model.py +++ b/sfic_solver/model.py @@ -264,6 +264,12 @@ def for_space(cls, space, **fields): fields.setdefault("master_min_span", span if span >= 1 else None) return cls(**fields) + def is_off(self): + """True if every rule is off, so every key passes.""" + return (self.max_run is None and self.max_same_depth is None + and not self.forbid_monotone and self.master_min_span is None + and self.min_total_variation is None) + def describe(self, rule, pins): """A phrase for a broken rule and its pins (from `violations`), saying where and never what the cuts are.""" diff --git a/sfic_solver/solve_system.py b/sfic_solver/solve_system.py index 25e4382..1681a76 100644 --- a/sfic_solver/solve_system.py +++ b/sfic_solver/solve_system.py @@ -59,8 +59,6 @@ # position lists up to 10 ** this digit combinations MAX_SHAPE_REJECTS = 20_000 # draws turned down by the shape rules before a key is given up # on (at least 50 for each candidate asked for) -SHAPE_OFF = model.ShapeRules(max_run=None, max_same_depth=None, forbid_monotone=False, - master_min_span=None, min_total_variation=None) UNPINNABLE_WEIGHT = 1.0 # one undecoded unit key that cannot take the master counts as # one expected cross-operation; with the scale of the two terms # it makes avoiding rekeyed unit cores the primary goal @@ -147,8 +145,8 @@ def __init__(self, cfg): # Shape rules: they apply to every key the solver chooses; a master is a key that is in # the masters of any core. `no_shape` holds the keys for which no bitting was found. self.shape = cfg.shape - self.shape_on = cfg.shape != SHAPE_OFF - self.masters = {m for core in cfg.cores for m in core["masters"]} + self.shape_on = not cfg.shape.is_off() + self.masters = cfg.master_keys self.no_shape = {} # key -> why no bitting follows the shape rules # Pinning: only for files that set it. The population of undecoded unit keys comes diff --git a/tests/test_check_system.py b/tests/test_check_system.py index 967a479..bdccc2a 100644 --- a/tests/test_check_system.py +++ b/tests/test_check_system.py @@ -431,3 +431,90 @@ def test_more_covering_retired_cores_than_the_limit_fall_back_to_the_uniform_pop "can be combined exactly." in section three = dict(pinned, retired_cores=pinned["retired_cores"][:3]) assert "sat in the retired core(s) Old 0, Old 1, Old 2" in residual(check(write_cfg, three).stdout) + + +# -- shape advice (docs/designs/key-shape-rules.md) ---------------------------------------- + +def shape_lines(proc): + return [line for line in proc.stdout.splitlines() if line.startswith("SHAPE")] + + +def test_shape_advice_is_listed_for_keys_that_break_the_rules(clean_cfg, write_cfg): + proc = run_script("check_system", write_cfg(clean_cfg)) + assert "== Key shape: advice only" in proc.stdout + lines = shape_lines(proc) + assert "SHAPE general_master: the cuts only go one way along the key " \ + "(never down, or never up)" in lines + assert "SHAPE control_common: a depth used more than 3 times at pin(s) [1, 3, 5, 7]" \ + in lines + + +def test_shape_advice_does_not_change_the_exit_status_or_the_counts(clean_cfg, write_cfg): + proc = run_script("check_system", write_cfg(clean_cfg)) + assert shape_lines(proc) and proc.returncode == 0 + assert proc.stdout.strip().splitlines()[-1] == "OK" # no problems, no warnings + + +def test_shape_advice_leaves_out_decoded_unit_keys_and_retired_keys(clean_cfg, write_cfg): + clean_cfg["keys"]["unit:104"] = "0123456" # monotone, and a unit key + clean_cfg["retired_keys"]["old_master"] = "2233445" # equal neighbours, and retired + proc = run_script("check_system", write_cfg(clean_cfg)) + advice = "\n".join(shape_lines(proc)) + assert "unit:" not in advice and "old_master" not in advice + + +def test_the_span_rule_applies_to_keys_that_are_masters_only(clean_cfg, write_cfg): + narrow = "2345432" # fits the pattern, spans only 3, one run of nothing + clean_cfg["keys"]["general_master"] = narrow + clean_cfg["keys"]["key_c"] = "4325214" # not a master; also narrow, and fine otherwise + proc = run_script("check_system", write_cfg(clean_cfg)) + advice = shape_lines(proc) + assert ("SHAPE general_master: a master whose deepest and shallowest cuts differ by " + "less than 6") in advice + assert not any("key_c" in line for line in advice) + + +def test_shape_advice_names_positions_and_never_cuts(clean_cfg, write_cfg): + clean_cfg["keys"]["key_c"] = "4325214" + clean_cfg["keys"]["key_b"] = "6743856" + proc = run_script("check_system", write_cfg(clean_cfg)) + for line in shape_lines(proc): + assert not any(c in line for c in ("4325214", "6743856", "2345678", "0123456")) + + +def test_the_files_shape_settings_change_the_advice(clean_cfg, write_cfg): + clean_cfg["shape"] = {"max_same_depth": None} + lines = shape_lines(run_script("check_system", write_cfg(clean_cfg))) + assert lines and not any("a depth used" in line for line in lines) + + +def test_with_every_rule_off_there_is_no_shape_section(clean_cfg, write_cfg): + clean_cfg["shape"] = {name: None for name in ("max_run", "max_same_depth", "forbid_monotone", + "master_min_span", "min_total_variation")} + proc = run_script("check_system", write_cfg(clean_cfg)) + assert "Key shape" not in proc.stdout and "SHAPE" not in proc.stdout + assert proc.returncode == 0 + + +def test_a_long_list_of_advice_is_capped_like_the_other_lists(clean_cfg, write_cfg): + import itertools + cuts = [c for c in itertools.combinations_with_replacement(range(10), 7) + if all(b - a <= 5 for a, b in zip(c, c[1:]))][:40] # 40 keys that only go up + clean_cfg.update(pattern=None, keys={f"k{i}": "".join(map(str, c)) for i, c in enumerate(cuts)}, + retired_keys={}, control_keys={}, cores=[], unit_count=None) + proc = run_script("check_system", write_cfg(clean_cfg)) + lines = proc.stdout.splitlines() + start = lines.index("== Key shape: advice only (decoded unit keys and retired keys are not " + "checked) ==") + shown = lines[start + 1:start + 31] + assert len(shown) == 30 and all(line.startswith("SHAPE") for line in shown) + assert re.fullmatch(r"\.\.\. and \d+ more", lines[start + 31]) + + +def test_decoded_unit_keys_are_exempt_from_shape_advice_but_not_from_parity( + clean_cfg, write_cfg): + """The exemption is for the SHAPE lines only: parity and MACS still apply to unit keys.""" + clean_cfg["keys"]["unit:104"] = "1000000" # wrong parity for EOEOEOE, and flat + proc = run_script("check_system", write_cfg(clean_cfg)) + assert "PARITY unit:104" in proc.stdout + assert not any("unit:104" in line for line in shape_lines(proc)) diff --git a/tests/test_config.py b/tests/test_config.py index 6caf8a5..a368b48 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -495,3 +495,9 @@ def test_any_number_of_retired_cores_covering_units_is_accepted_by_the_loader(pi pinned["retired_cores"] = [{**template, "name": f"Old {n}"} for n in range(8)] cfg = parse_config(pinned) # the checker, not the loader, limits what it combines assert len(cfg.retired_cores) == 8 and all(c["covers_units"] for c in cfg.retired_cores) + + +def test_master_keys_are_the_keys_in_the_masters_of_any_core(clean_cfg): + cfg = parse_config(clean_cfg) + assert cfg.master_keys == {"general_master", "unit_master"} + assert parse_config({**clean_cfg, "cores": []}).master_keys == set() diff --git a/tests/test_model.py b/tests/test_model.py index 87894ee..1848f81 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -454,3 +454,13 @@ def test_unmeetable_names_a_rule_no_key_could_meet(): "than 4 (max_step is 1)") off = model.ShapeRules(master_min_span=None, min_total_variation=None) assert off.unmeetable(space) is None + + +def test_is_off_only_when_every_rule_is_off(): + off = dict(max_run=None, max_same_depth=None, forbid_monotone=False, master_min_span=None, + min_total_variation=None) + assert model.ShapeRules(**off).is_off() + assert not model.ShapeRules().is_off() + for field, on in (("max_run", 1), ("max_same_depth", 3), ("forbid_monotone", True), + ("master_min_span", 6), ("min_total_variation", 5)): + assert not model.ShapeRules(**{**off, field: on}).is_off(), field