Repository navigation
Research: five post-certificate structural improvements and executed benchmarks - #33
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 572b98b7ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for bits in product((0,1),repeat=len(atoms)): | ||
| endpoints.append({a:intervals[a][bits[i]] for i,a in enumerate(atoms)}) |
There was a problem hiding this comment.
Expected residual variance need not attain its maximum at an interval endpoint: for three singleton queries with budget 2 and intervals 0=[.1,.9], 1=[.3,.4], and 2=[.5,.5], this enumeration selects review (1,2) and reports worst case .09, while the allowed interior value p0=.5 yields .25; reviewing (0,2) has a true worst case of .24. Since oracle_interval_worst repeats the same endpoint restriction, the reported zero discrepancies do not detect this, and the H4 minimax/never-worse conclusions are not established for the supplied probability intervals.
Useful? React with 👍 / 👎.
| for node,(op,deps) in self.definitions.items(): | ||
| for d in deps: | ||
| self.reverse[d].add(node) | ||
| self.derived_order=tuple(sorted(self.definitions)) |
There was a problem hiding this comment.
Sorting derived identifiers is topological only when identifiers happen to encode dependency order, although DependencyGraph otherwise accepts any acyclic dependency graph. For example, with primitive 0 and definitions 1 <- 2 and 2 <- 0, full_recompute reads node 2 before it exists, while transact can read node 2's old value before recomputing it and return an inconsistent snapshot. Compute and validate an actual topological ordering rather than relying on the benchmark's increasing node numbering.
Useful? React with 👍 / 👎.
| run: | | ||
| set -euo pipefail | ||
| protocol=8187cc4e4b98e40f3146ce787b71f262b62b3abd | ||
| git merge-base --is-ancestor "$protocol" HEAD |
There was a problem hiding this comment.
This verification deterministically fails for the reviewed commit: git merge-base --is-ancestor 8187cc4e4b98e40f3146ce787b71f262b62b3abd 7e79b3a54a68b822df0a27c1c7b6320d00d4680b returns 1 because the commit's sole parent is baseline 9d2e4a6, not the frozen protocol commit. Although the copied protocol bytes match, the required pre-execution ancestry is absent, so every PR run exits at this step before reaching the tests; include the protocol commit in the history rather than only copying its file.
Useful? React with 👍 / 👎.
| scope=tuple(sorted(set(raw))) | ||
| if len(scope)<2: | ||
| raise ValueError('forbidden scope must have at least 2 vertices') | ||
| table={bits:(NEG_INF if all(bits) else 0) for bits in product((0,1), repeat=len(scope))} |
There was a problem hiding this comment.
A forbidden scope is supposed to stage when it exceeds the width/scope cap, but its complete 2^len(scope) table is allocated before the width check runs. For example, a valid call containing one 30-vertex forbidden hyperedge attempts to create over a billion dictionary entries before it can return staged, likely exhausting memory and defeating the stated safety cap. Normalize and reject over-width scopes before materializing their factor tables.
Useful? React with 👍 / 👎.
| ['H1','met','factor-state reduction',f"{h1['state_reduction']:.12f}",'counted DP states, not wall-clock'], | ||
| ['H2','met','local evaluation reduction',f"{h2['evaluation_reduction']:.12f}",'connected control saves 0%'], | ||
| ['H3','met','small/large oracle failures','0','supplied lineage and costs only'], | ||
| ['H4','met','strict minimax improvements',str(h4['strict_improvements']),'independent marginals with interval uncertainty'], | ||
| ['H5','met','large exact hypergraph fixtures',str(len(h5['large'])),'supplied n-ary constraints and utility'], |
There was a problem hiding this comment.
Every summary outcome is hard-coded as met rather than derived from primary_target_met. If a corrected implementation or replay records any target as false, report generation will still publish that hypothesis as met—in the CSV, table, abstract, and current-results snippet—contradicting the frozen requirement to retain failed conjunctions as failures. Generate these labels and surrounding claims from the recorded booleans.
Useful? React with 👍 / 👎.
| assert sha('manuscript/paper-current.pdf')==b['pdf_sha256'] | ||
| print('Committed current-paper hashes verified') | ||
| PY | ||
| python -B -m graph_synthesis.render_current_paper --output "$RUNNER_TEMP/post-certificate-paper.pdf" |
There was a problem hiding this comment.
In the pull_request path, this render command writes the independently rendered PDF to the temporary directory but render_current_paper.render() always rewrites manuscript/paper-current.html in the worktree. A PR can therefore commit stale HTML and pass because the later git diff --check only warns about conflict markers and whitespace errors, not ordinary uncommitted changes. Check the deterministic HTML before overwriting it, or fail on a nonempty worktree diff after rendering.
Useful? React with 👍 / 👎.
| try: g.transact(values,rev,changes,expected_revision=rev,fail_node=fail) | ||
| except RuntimeError: pass | ||
| if values==before and rev==brevis: failure_atomic+=1 |
There was a problem hiding this comment.
These controls increment their success counters whenever the original values and integer rev remain unchanged, without recording whether transact actually raised. Because transact operates on a copied overlay and each successful return is discarded here, the condition is also true if injected failures, stale revisions, or malformed updates are incorrectly accepted (and if fail is None), so the benchmark can still report 64/64 atomic controls. Track the expected exception explicitly and count the control only when it was raised and the inputs remained unchanged.
Useful? React with 👍 / 👎.
| # window, so 0..n-1 is the same endpoint-first min-fill elimination. | ||
| nonunary=[f.scope for f in factors if len(f.scope)>1] | ||
| if n>64 and nonunary and all(max(s)-min(s)<=2 for s in nonunary): | ||
| order=list(range(n)); width=max(max(s)-min(s) for s in nonunary) |
There was a problem hiding this comment.
The maximum numeric span of a scope is not the induced width of the selected elimination order. For example, a 65-vertex graph containing only forbidden pairs (i, i+2) has width 1 under the natural order, but this branch records width 2 and incorrectly returns staged when called with width_cap=1. Compute the maximum later-neighbor scope produced by the chosen order rather than using vertex-label distance.
Useful? React with 👍 / 👎.
| ['H4','met','strict minimax improvements',str(h4['strict_improvements']),'independent marginals with interval uncertainty'], | ||
| ['H5','met','large exact hypergraph fixtures',str(len(h5['large'])),'supplied n-ary constraints and utility'], | ||
| ] | ||
| files['summary.csv']='\n'.join(','.join(row) for row in summary)+'\n' |
There was a problem hiding this comment.
The summary is serialized with raw comma joining even though the H1 boundary value itself contains a comma (counted DP states, not wall-clock). Consequently, the committed H1 row has six fields while the header and other rows have five, so standard CSV readers shift or reject that record. Use the imported csv module to quote fields when producing this artifact.
Useful? React with 👍 / 👎.
CompleteDotTech
added
reproducibility
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e197e250c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| got=select_endpoint_minimax(queries,intervals,2) | ||
| om=oracle_endpoint_worst(queries,intervals,got['minimax']) | ||
| op=oracle_endpoint_worst(queries,intervals,got['midpoint']) | ||
| if abs(om-got['minimax_worst'])>1e-10 or abs(op-got['midpoint_worst'])>1e-10: discrepancies+=1 |
There was a problem hiding this comment.
For each fixture, the independent oracle is invoked only for the production-selected minimax and midpoint pairs, even though got['all_worst'] contains all 15 candidate-pair scores and the frozen H4 test requires every candidate pair to be checked. An incorrect Shannon score for any other pair can change which pair is selected while these two comparisons still pass, allowing oracle_discrepancies == 0 and a met H4 target without validating the minimization itself; compare every candidate's production score against its oracle score.
Useful? React with 👍 / 👎.
| def _solve_hitting_component(clauses: Sequence[FrozenSet[int]], costs: Mapping[int,int]) -> Tuple[int,Set[int]]: | ||
| clauses=_normalize_clauses(clauses) | ||
| best_cost=10**18; best_set:set[int]=set() |
There was a problem hiding this comment.
Positive integer removal costs have no documented upper bound, but 10**18 is treated as an incumbent rather than infinity. For the valid input clauses=[(0,)] and costs={0: 10**18}, the root lower bound equals this sentinel, so the search prunes before recording a solution and query_resilience subsequently raises AssertionError: invalid deletion witness; initialize the incumbent with inf instead.
Useful? React with 👍 / 👎.
| output_states=1<<len(union) | ||
| if materialized+output_states>state_cap or states+2*output_states>state_cap: | ||
| return EliminationResult('staged',None,tuple(),tuple(order),width,states) |
There was a problem hiding this comment.
The second condition stages based on states, which counts elimination evaluations rather than the documented cumulative materialized factor-table states. For example, ten unit-weight vertices with scopes [(3,5),(9,4),(6,5,2,7,4),(3,1),(8,6,1),(3,9),(7,9,8)] have width 4 and exactly 172 cumulative input/output table entries, yet state_cap=172 stages after 170 evaluations while state_cap=176 solves; thus a component satisfying the advertised allocation cap can be rejected by an additional, undocumented work-count cap.
Useful? React with 👍 / 👎.
| for p,v in changes.items(): | ||
| if not isinstance(p,int) or p<0 or p>=self.primitive_count or not isinstance(v,bool): | ||
| raise ValueError('malformed update') |
There was a problem hiding this comment.
Python booleans satisfy isinstance(p, int), so a malformed update such as {False: True} passes this validation and is committed as an update to primitive 0 (including recomputing its descendants and advancing the revision). Derived identifiers are deliberately validated with type(node) is int, and the transaction contract requires every primitive change to be validated, so use the same strict integer check here to prevent Boolean input from aliasing a real primitive.
Useful? React with 👍 / 👎.
1 participant
Scope and protocol
Five controlled post-certificate structural studies. The frozen protocol was committed before implementation at
8187cc4e4b98e40f3146ce787b71f262b62b3abd; its bytes and ancestry are preserved. Merge with a merge commit to retain that history.No fresh Jev or other paid model calls were made. Results are conditional algorithm tests, not semantic accuracy or production claims.
Corrected outcomes
See
graph_synthesis/post_certificate/CORRECTIONS.mdfor the explicit correction record,RESULTS.mdandresults.jsonfor measured details, and the updated current manuscript. The original protocol, fixture populations, seeds, caps, and thresholds were not retuned.Validation
The earlier PR body and hosted check described five passes. Those claims are superseded by this corrected audit. No automatic PR checks are expected after the repository's CI policy change.