Skip to content

Fix the two failures keeping master's CI red - #153

Open
yichao-liang wants to merge 2 commits into
masterfrom
ci-green-dill-pin
Open

Fix the two failures keeping master's CI red#153
yichao-liang wants to merge 2 commits into
masterfrom
ci-green-dill-pin

Conversation

@yichao-liang

Copy link
Copy Markdown
Collaborator

Master's CI has two distinct failures. This fixes both.

1. TypeError: cannot pickle '_abc._abc_data' object

The dominant one - it has failed whichever pytest-split shard test_main,
test_degenerate_mlp_sampler_learning, test_oracle_samplers or the GNN
suites landed on, three times in two days (runs 32418799832, 32260627910,
32258416611).

It is not collection timing, which is what #142 guessed. It is the
dill==0.3.5.1 pin. dill serialises a class by value whenever it cannot
find that exact class again at module.qualname, and before 0.3.6 the
by-value path shipped the class __dict__ verbatim - including the
_abc_impl slot ABCMeta puts on every class it builds, which holds an
unpicklable _abc_data. dill 0.3.6 added _get_typedict_abc, which strips
_abc_impl and the ABC registry caches and re-registers the subclasses on
load. Nothing else in an object graph can reach an _abc_data, so that path
is the whole bug. It reproduces directly on the pinned version and not on
0.3.9.

dill==0.3.9 pulls multiprocess 0.70.17 / pathos 0.3.3, its lockstep
releases. #142's retry is removed - it was a mitigation on a hypothesis its
own docstring said the continued failures would rule out - but the
serialise-before-writing it was built on stays, so pkl_dump_with_retry
becomes pkl_dump_all_or_nothing. Its "fake a transient failure" test is
replaced by one that pickles a genuinely by-value ABC, so a pin that slips
back below 0.3.6 fails there instead of at random in CI.

2. test_emcee_recovers_rate_params

It asserted rel_err < 0.3 on FitResult.point_estimate, which is the
single highest-log-probability draw out of 9600. The data does not identify
the rate params that sharply - water_fill_speed's 95% credible interval
runs [0.0031, 0.0277] around a true 0.0200, roughly ±70% wide - so which
draw wins the argmax moves between machines, and PyBullet trajectory
generation is platform-dependent on top of that. The same commit passed on
master's runner and failed on another at 32.2%.

Chain length is not the lever: 500 and 1000 steps return bit-identical
estimates. Percentiles of that chain are stable where the argmax is not, so
they are what gets asserted now - truth inside the 95% interval, and the
posterior median closing at least a quarter of the gap the 50% perturbation
opened.

Verification

On this branch (master + these two commits):

  • 362 passed, 3 skipped across every dill-touching test file, all three GNN
    approach suites, tests/code_sim_learning/ and tests/test_main.py
  • mypy: Success: no issues found in 724 source files
  • yapf / isort / docformatter clean; pylint 10.00/10 on the changed files

One process note

master requires 14 checks and one review, but enforce_admins is off, so
gh pr merge --admin clears a failing check and not just the review gate.
That is how the three red merges above landed. Worth turning
enforce_admins on, or agreeing not to use --admin on anything red.

`TypeError: cannot pickle '_abc._abc_data' object` has been failing
whichever pytest-split shard test_main, test_degenerate_mlp_sampler_-
learning, test_oracle_samplers or the GNN suites landed on, often
enough that master merged red three times in two days. #142 read it as
collection timing and added a collect-and-retry; that docstring said
outright that if the failures continued the hypothesis was ruled out.
They continued.

The cause is the pin. dill serialises a class BY VALUE whenever it
cannot find that exact class again at module.qualname, and before 0.3.6
the by-value path shipped the class __dict__ verbatim - including the
_abc_impl slot ABCMeta puts on every class it builds, which holds an
unpicklable _abc_data. dill 0.3.6 added _get_typedict_abc, which strips
_abc_impl and the ABC registry caches and re-registers the subclasses
on load. It reproduces directly: dill 0.3.5.1 raises on dumping a class
it has to write by value, 0.3.9 returns the bytes. Nothing else in an
object graph can reach an _abc_data, so that path is the whole bug.

0.3.9 pulls multiprocess 0.70.17 / pathos 0.3.3, its lockstep releases.

The retry goes with it. It is a disproven mitigation, and leaving it
would only mask the next real diagnosis. What it was built on stays: a
dump that raises part-way has already written a prefix, and a prefix is
a pickle that fails at LOAD time, long after the run that made it could
have been repeated. So pkl_dump_with_retry becomes
pkl_dump_all_or_nothing, and its "fake a transient failure" test is
replaced by one that pickles a genuinely by-value ABC - a pin that
slips back below 0.3.6 now fails there instead of at random in CI.

Claude-Session: https://claude.ai/code/session_01QTCeKe2iqBn4rCCdAYQWtX
test_emcee_recovers_rate_params asserted rel_err < 0.3 on
FitResult.point_estimate, which is the single highest-log-probability
draw out of 9600. This data does not identify the rate params that
sharply: water_fill_speed's 95% credible interval runs [0.0031, 0.0277]
around a true 0.0200, roughly +-70% wide. Which draw wins the argmax
therefore moves between machines, and PyBullet trajectory generation is
platform-dependent on top of that, so the transitions feeding the chain
differ too. The same commit passed on master's runner and failed on
another at 32.2%, two points past a threshold the posterior never
supported.

Chain length is not the lever: 500 and 1000 steps return bit-identical
estimates, so the sampler has already found what there is to find.

Percentiles of that same chain are stable where the argmax is not, so
they are what gets asserted - the truth has to sit inside the 95%
interval, and the posterior median has to close at least a quarter of
the gap the 50% perturbation opened. A fit that stopped working fails
both: a chain that never left init misses the gap closure, and one that
converged on the wrong value puts the truth outside the interval.

Claude-Session: https://claude.ai/code/session_01QTCeKe2iqBn4rCCdAYQWtX
@yichao-liang
yichao-liang enabled auto-merge (squash) August 26, 2026 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant