Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .changeset/gate-order-is-a-selection.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
'@platforma-open/milaboratories.sort-seq-analysis.model': patch
'@platforma-open/milaboratories.sort-seq-analysis.software': patch
'@platforma-open/milaboratories.sort-seq-analysis.ui': patch
'@platforma-open/milaboratories.sort-seq-analysis.block': patch
---

Gate order is a selection, not a ranking of every value the gate column carries

The block no longer refuses to run until every distinct value of the gate column has been
given an order position. The gate column of a real sort-seq run routinely carries values
that are not rungs on the binding ladder — an unsorted input, a specificity arm, a
stability arm — and demanding a rank for each refused configurations the computation runs
perfectly well.

The ordered list is now the run's gate scope: the gates it holds, in the order it holds
them, are the ladder, and removing a value takes it and its samples out of the run. Ranks
stay contiguous from 1 over the gates that remain, so a removal leaves no gap that would
shift every score.

- **Model** — the coverage check is gone; what remains is that the list is non-empty and
names nothing the column does not carry.
- **Computation** — rows outside the declared ladder are dropped before the depths are
taken, so an unselected gate contributes to neither sum of the weighted mean. Its
samples are likewise outside the one-sample-per-group and sort-fraction refusals, and
its fraction is not part of a condition's sum.
- **A condition whose every sample sits in an unselected gate** is dropped from the run,
exactly as an excluded condition is, rather than scored to an empty file.
9 changes: 0 additions & 9 deletions docs/description.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,3 @@ A run over N conditions emits both quantities N times, once per condition. A one
ordinary run: it gets both quantities, with the run's single condition on each column exactly as a
two-condition run would carry two.

## Downstream

Everything downstream of this block is a comparison of these scores — a pH switch is the difference between
a variant's bin score at two pH arms, the on-state is its raw score at the arm the campaign treats as *on*,
and a shortlist is a ranking over one of them.

## Status

Under development. The specification lives in `docs/text/work/projects/sequence-repertoires/facs-bin-analysis/`.
29 changes: 19 additions & 10 deletions model/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,27 @@ export function settingsIssues(data: BlockData): string[] {
issues.push("The condition, gate and sort-fraction columns must be three different columns");
}

// 4 incomplete — the order must cover exactly the gate column's values. The drag list makes
// duplicates and unknown values unrepresentable, so what is left to check is that the
// snapshot and the order agree; they can drift if upstream re-emits the column and the user
// does not re-pick it.
// 4 the order is a **selection**, not a ranking of everything the column carries. The
// gates it lists, in the order it lists them, are the run's binding ladder; a gate the
// user removed is not part of the run at all, exactly as an excluded condition is not.
//
// So coverage is deliberately not checked. A gate column carrying values that are not
// rungs on this ladder — an unsorted input, a specificity arm, a stability arm — is the
// ordinary case for a sort-seq run, and demanding a rank for each would refuse a
// configuration the computation runs perfectly well.
//
// What is left is that the list is not empty, and that it names nothing the column does
// not carry; the latter can drift if upstream re-emits the column and the user does not
// re-pick it.
// Only once a gate column is picked. Collecting every issue rather than throwing on the
// first means an unguarded check here would tell a freshly added block that "the gate column
// has no values to rank" while also telling it to select a gate column — two complaints for
// one unmade choice.
if (data.gateColumnRef !== undefined) {
if (data.gateValues.length === 0) {
issues.push("The gate column has no values to rank");
}
const missing = data.gateValues.filter((value) => !data.gateOrder.includes(value));
if (missing.length > 0) {
issues.push(
`Assign an order position to every gate: ${missing.join(", ")} ${missing.length === 1 ? "is" : "are"} missing`,
);
} else if (data.gateOrder.length === 0) {
issues.push("Keep at least one gate in the order — every gate has been removed");
}
const unknown = data.gateOrder.filter((value) => !data.gateValues.includes(value));
if (unknown.length > 0) {
Expand Down Expand Up @@ -146,6 +150,11 @@ export const platforma = BlockModelV3.create(blockDataModel)
// same bytes and does not fire the staleness gate.
// Position becomes the rank: first in the list is gate 1, the weakest binder. The
// computation weights by these integers, so the list's order is the whole signal.
// Ranks are contiguous over the gates the list actually holds — a removed gate leaves
// no gap, because the ladder is the selection and rank values enter the weighted mean
// as numbers. A gap would move every score without naming a reason.
// The map is also what tells the computation which gates the run covers: a gate absent
// from it is dropped along with its samples.
gateRanks: Object.fromEntries(data.gateOrder.map((gate, index) => [gate, index + 1])),
excludedConditions: [...data.excludedConditions].sort(),
// Both optional arguments are passed through as `undefined` when unset rather than
Expand Down
13 changes: 11 additions & 2 deletions model/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,15 @@ export type BlockArgs = {
/** 3. A per-sample metadata column, picked for the other role. */
gateColumnRef: SUniversalPColumnId;
/**
* 4. A rank per distinct value of the gate column, along the binding axis.
* 4. A rank per **selected** gate, along the binding axis. Ranks are contiguous from 1.
*
* Derived from `BlockData.gateOrder` by position — the workflow and the computation want a
* value → rank map, the user wants to drag a list.
*
* The key set doubles as the run's gate scope: a value of the gate column absent from this
* map is not a rung on the ladder, and its samples take no part in the run. A gate column
* that also names an unsorted input or a specificity arm is the ordinary case, not an
* incomplete configuration.
*/
gateRanks: Record<string, number>;
/** 5. Absent (empty) means every distinct value of the condition column is a condition. */
Expand All @@ -38,11 +43,15 @@ export type BlockData = {
conditionColumnRef?: SUniversalPColumnId;
gateColumnRef?: SUniversalPColumnId;
/**
* The gate values in declared order, weakest binder first.
* The selected gates in declared order, weakest binder first.
*
* A list rather than a value → rank map because the control is drag-to-reorder and position
* *is* the rank, which makes a duplicated rank and a rank naming an absent value
* unrepresentable rather than merely refused.
*
* Seeded with every value the gate column carries when the column is picked, and then
* **narrowed by the user**: removing a gate takes it out of the run. It need not cover
* `gateValues`, only be non-empty and name nothing outside it.
*/
gateOrder: string[];
excludedConditions: string[];
Expand Down
5 changes: 4 additions & 1 deletion software/src/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

Shape is fixed by `computation-interface`:

gateRanks each distinct value of the gate column -> its integer rank
gateRanks each *selected* gate value -> its integer rank, contiguous from 1.
The key set is the run's gate scope: a value of the gate column
absent from it is not a rung on the ladder and its samples are
dropped. Coverage of the column is neither required nor checked.
excludedConditions condition values to drop; empty where none are excluded
readFloor a non-negative integer, or null for no floor
sortFractionColumn the reads-table column carrying frac_cb, or null for uncorrected
Expand Down
47 changes: 41 additions & 6 deletions software/src/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,22 @@

def run(reads: pl.DataFrame, variants: pl.DataFrame | None, params: Params, out_dir: Path) -> dict:
"""Score every retained condition and write every file. Returns the manifest."""
retained = retained_conditions(reads, params)
in_scope = selected_gates(reads, params)
retained = retained_conditions(in_scope, params)

# Both refusals run over the whole run before anything is written, so a failure leaves
# nothing partial behind.
check_one_sample_per_group(reads, retained)
# nothing partial behind. Over the in-scope rows only: a second sample in a gate the run
# does not cover, or a missing sort fraction on one, is not this run's problem.
check_one_sample_per_group(in_scope, retained)
if params.sort_fraction_column is not None:
check_sort_fractions(reads, params.sort_fraction_column, retained)
check_sort_fractions(in_scope, params.sort_fraction_column, retained)

parent = scoring.resolve_parent(variants)

out_dir.mkdir(parents=True, exist_ok=True)

conditions = [
_score_one_condition(reads, params, parent, condition, index, out_dir)
_score_one_condition(in_scope, params, parent, condition, index, out_dir)
for index, condition in enumerate(retained)
]

Expand All @@ -65,12 +67,38 @@ def run(reads: pl.DataFrame, variants: pl.DataFrame | None, params: Params, out_
return manifest


def selected_gates(reads: pl.DataFrame, params: Params) -> pl.DataFrame:
"""The rows whose gate is a rung on the declared ladder, and no others.

`gateRanks` is a **selection**: the caller ranks the gates the run covers and says
nothing about the rest. A gate column routinely carries values that are not rungs — an
unsorted input, a specificity arm, a stability arm — and their samples take no part in
the arithmetic, are not depths, and are not gates a distribution draws.

Dropping them here, once, rather than at each use is what lets every function downstream
keep reading `gate_ranks` as total over the rows it is handed: the `replace_strict` in
`scoring.gate_rank_means` and the sort keys in `_gates_collected` and
`read_distribution` all become internal invariants guarded by this one filter.

Filtering **before** the depths are taken is required, not incidental: `depth_cb` sums
over a gate's own rows, so an unselected gate cannot change a selected gate's
frequencies — but it would otherwise contribute a rank-less term to both sums of the
weighted mean, which is precisely the arithmetic clause 2 confines to collected gates.
"""
return reads.filter(pl.col(COL_GATE).is_in(list(params.gate_ranks)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Unmatched gates produce empty success

When a direct CLI invocation supplies a non-empty gateRanks map whose keys match none of the current gate values, selected_gates removes every row and the pipeline exits successfully with an empty conditions manifest and no score files instead of rejecting the stale selection.

Prompt To Fix With AI
This is a comment left during a code review.
Path: software/src/pipeline.py
Line: 88

Comment:
**Unmatched gates produce empty success**

When a direct CLI invocation supplies a non-empty `gateRanks` map whose keys match none of the current gate values, `selected_gates` removes every row and the pipeline exits successfully with an empty `conditions` manifest and no score files instead of rejecting the stale selection.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code



def retained_conditions(reads: pl.DataFrame, params: Params) -> list[str]:
"""The condition column's own distinct values, minus the excluded ones, sorted.

The values are the column's, never a set the caller typed: a typo then becomes a value
matching no sample rather than a silent second condition.

Read from the **in-scope** rows, so a condition whose every sample sits in an unselected
gate is not a condition of this run — it is dropped exactly as an excluded value is,
rather than scored to an empty file. An empty result is indistinguishable from a failed
one, and the gate selection is as much a scoping argument as the exclusion list.

Sorting is not an ordering claim — conditions carry none (`condition-source`), and
nothing this block emits depends on their order. It is here because the sort makes each
condition's file index deterministic, which the workflow's pure-template dedup needs.
Expand Down Expand Up @@ -147,6 +175,10 @@ def _gates_collected(slice_c: pl.DataFrame, gate_ranks: dict[str, int]) -> list[
which clause 1 handles by having it contribute to neither sum. A gate with no sample at
this condition is simply not collected here, and that is not an error.

Only selected gates can appear — the slice is already filtered — so the summary reads as
the ladder the run actually used, and an unsorted-input or specificity sample never shows
up as a rung with a large depth beside the real gates.

Ordered by declared rank, so the run summary reads along the binding axis.
"""
depths = slice_c.group_by(COL_GATE).agg(pl.col(COL_READS).sum().alias("depth"))
Expand All @@ -166,7 +198,10 @@ def _sort_fraction_sum(slice_c: pl.DataFrame, sort_fraction_column: str | None)
supplies one fraction per gate.

A sum short of 1.0 is legitimate and is not renormalized: it is what a condition that
collected only some of the declared gates correctly looks like.
collected only some of the declared gates correctly looks like — and, now that the
ladder is a selection, what a run covering only some of a column's gates looks like
too. The fractions of unselected gates are not summed in, because those gates supplied
nothing the weighted mean used.
"""
if sort_fraction_column is None:
return None
Expand Down
9 changes: 5 additions & 4 deletions software/src/scoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,11 @@ def gate_rank_means(per_gate: pl.DataFrame, gate_ranks: dict[str, int]) -> pl.Da
fraction is never reconstructed.
"""
# replace_strict raises on a gate value with no rank rather than dropping its reads.
# An incomplete gate order is a *configuration* violation the block model refuses
# before the run (`argument-surface` rule 4), and `validation-boundary` forbids
# checking one rule in two places — so this is not a second check but an internal
# invariant, guarding a direct CLI caller against silently losing a gate.
# Reaching it is a bug in this package, not a caller error: `gateRanks` names the gates
# the run covers, and `pipeline.selected_gates` has already dropped every row outside
# that set. So this is an internal invariant — the one place that would notice a future
# caller assembling `per_gate` without going through that filter, where the failure
# would otherwise be a silently lighter weighted mean.
rank = pl.col(COL_GATE).replace_strict(gate_ranks, return_dtype=pl.Float64)

return (
Expand Down
6 changes: 4 additions & 2 deletions software/src/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
1, and whether two samples share a gate, are properties of the project's data.

Every configuration rule — a required argument absent, an anchor resolving to nothing or
to more than one column, the three metadata roles not distinct, an incomplete gate order,
to more than one column, the three metadata roles not distinct, an empty gate order,
every condition excluded, a negative floor — is refused by the block model before the run
starts and is deliberately **not** re-checked here. Two implementations of one rule, in
two languages, either changeable alone, is a rule that will disagree; the failure mode is
Expand Down Expand Up @@ -46,7 +46,9 @@ def check_one_sample_per_group(reads: pl.DataFrame, retained_conditions: list[st
No sample for a pair is **not** an error — that gate was not collected at that
condition, which clauses 1 and 2 already accommodate.

Only retained conditions are checked; an excluded value is not part of the run.
Only retained conditions are checked; an excluded value is not part of the run. The
caller likewise hands over only the selected gates' rows, so two samples sharing a gate
the run does not cover pass unremarked — that pair is not a group of this run.
"""
offenders = (
reads.filter(pl.col(COL_CONDITION).is_in(retained_conditions))
Expand Down
Loading
Loading