Fix Difference hash seed widening on 32-bit (UInt == UInt32) - #688
Draft
ChrisRackauckas-Claude wants to merge 1 commit into
Draft
ChrisRackauckas-Claude wants to merge 1 commit into
ChrisRackauckas-Claude wants to merge 1 commit into
Conversation
Base.hash(::Difference, ::UInt) xored the native-width seed `u` against a bare literal (0x055640d6d952f101), which only fits UInt64. On 32-bit builds, where UInt == UInt32, this silently widens the xor result to UInt64, so the subsequent hash(D.t, ::UInt64) call on the SymbolicUtils term has no matching method (SymbolicUtils's hashconsing expects the platform's native UInt). Reducing the literal with `% UInt` keeps the result in the caller's native width on every platform. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Agent-Harness: Claude Code Agent-Model: claude-sonnet-5-5 Agent-Session: subagent of the qa-hygiene head session on amdci2
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ignore until reviewed by @ChrisRackauckas.
Summary
Base.hash(D::Difference, u::UInt)(src/difference.jl:41) xored the native-width seeduagainst a bare literal,0x055640d6d952f101, which does not fit inUInt32. On64-bit builds
UInt == UInt64so the literal fits and nothing goes wrong, but on 32-bitbuilds (
UInt == UInt32) thexorsilently widens the result toUInt64. The nextline then calls
hash(D.t, ::UInt64)on aSymbolicUtilsterm, but SymbolicUtils'shashconsing (
hash_bsimpl) is defined for the platform's nativeUInt—UInt32here —so there is no matching method and it throws a
MethodError.Fix: reduce the literal with
% UIntso thexorresult stays in the caller's nativewidth on every platform:
This is the bug behind the master
Core 32-bitCI failure inhttps://github.com/SciML/DataDrivenDiffEq.jl/actions/runs/35188714123 (job
Core (julia 1, ubuntu-latest, x86), two failures intest/Core/implicit_basis.jl).It is unrelated to #680 (which drops the x86 CI lane over a BFloat16s/NNlib
LLVM-on-i686 build error) — BFloat16s and NNlib both installed and precompiled cleanly
in that run; this failure is a genuine
DataDrivenDiffEqhashing bug in test content,not a dependency build problem.
Verification
All of this was run against real 32-bit Julia (
juliaup add 1~x86, native i686 on thisx86_64 host, matching CI's
julia-1.13.0+0.x86) in addition to the normal 64-bit build.1. Added test —
test/Core/implicit_basis.jl, right after theDifferencethattriggers the bug:
Red (before the fix, on x86 Julia 1.13.0) —
GROUP=Core julia +1~x86 --project -e 'using Pkg; Pkg.test()':(The original two assertions at what are now lines 63-64 fail with the identical
MethodErrorseen in the CI run linked above.)Green (after the fix, same x86 Julia 1.13.0) — same command:
2.
GROUP=Coreonce on the default (64-bit) Julia, with the fix applied:3. Runic / typos on the changed files, both clean:
What this does not fix
The x86
GROUP=Corerun (both red and green) also hits a separate, pre-existing,unrelated 32-bit failure in
test/Core/utils.jl(Optimal Shrinkage):MethodError: no method matching optimal_svht(::Int32, ::Int32)—optimal_svhtis onlydefined for
Int64arguments (src/utils/utils.jl:6). That is a different 32-bit bug andout of scope for this PR; not touched here.
Risk assessment
Differenceoperator type used onlyfor discrete-time difference equations; no public API signature changes, result values
for
hashon 64-bit builds are unchanged (literal already fitsUInt64there).implicit_basis.jlgreen on x86 after the fix; fullGROUP=Coregreen on x64; on x86,test/Core/utils.jlstill fails on the separate
optimal_svht(::Int32, ::Int32)bug described above; Runic/typos clean.hashon real x64 and x86 Julia 1.13.0: on x64 the hashes are identical, and on x86 the old one throws while the new one returnsUInt32. It noted that the new test only discriminates on 32-bit CI, and caught the x86 wording fixed above. Cursor auto is a first-pass reviewer, not a top reviewer.(distinct from the unrelated dependency-build issue Remove x86 CI lane (BFloat16s i686) #680 addresses)
🤖 Generated with Claude Code (model: claude-sonnet-5-5)
https://claude.ai/code/session_01QgnmpexRwud38wrs3XvBfV