Remove module-level RNG reset from gnn.py - #1227
Open
lehendo wants to merge 1 commit into
Open
Conversation
pyhealth/models/gnn.py called torch.manual_seed(3) and np.random.seed(1) at module level (right after its imports). Since pyhealth/models/__init__.py does 'from .gnn import GAT, GCN', this ran as a side effect of any 'import pyhealth.models' -- silently overwriting whatever seed the caller had already set for their own script's reproducibility, regardless of whether GCN/GAT were ever used. This is a process-wide reproducibility hazard: a user who does 'torch.manual_seed(42); import pyhealth.models; ...' (a completely standard, recommended pattern) would have their seed silently reset to 3 partway through, with no error or warning. Confirmed nothing else in the module depends on this running at import time -- everything else in the file is function/class definitions, whose bodies don't execute until called. Fix: delete both lines. Added a regression test that spawns a fresh subprocess (necessary since pyhealth.models is already imported -- and the bug already fired -- by the time any in-process test runs), seeds torch/numpy, records the next random draw, re-seeds identically, imports pyhealth.models, and checks the next draw is unchanged. Confirmed this test fails against the pre-fix code with the exact expected symptom (RNG stream diverges after the import) and passes after the fix; full existing GCN/GAT test suite (21/21) still passes, confirming the removal has no other effect.
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
1 participant
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.
pyhealth/models/gnn.py called torch.manual_seed(3) and np.random.seed(1) at module level. Since pyhealth/models/init.py imports from this file, simply running import pyhealth.models silently overwrote any seed a caller had already set.
The fix was to remove both module-level seed calls, which has been done.