diff --git a/docs/api/models/pyhealth.models.GNN.rst b/docs/api/models/pyhealth.models.GNN.rst index d7e08a9fd..7befca0e5 100644 --- a/docs/api/models/pyhealth.models.GNN.rst +++ b/docs/api/models/pyhealth.models.GNN.rst @@ -3,6 +3,17 @@ The GNN model (pyhealth trainer does not apply to GNN, refer to the example/ChestXray-image-generation-GAN.ipynb for examples of using GNN model). +.. note:: + + ``pyhealth.models.gnn`` no longer seeds the global ``torch``/``numpy`` + RNGs at import time. Previously, ``torch.manual_seed(3)`` and + ``np.random.seed(1)`` ran as module-level statements, so simply + importing ``pyhealth.models`` (which imports this module) would + silently overwrite any seed the caller had already set, regardless of + whether GCN/GAT were ever used. If you need reproducible GCN/GAT + weight initialization, seed explicitly right before constructing the + model instead. + .. autoclass:: pyhealth.models.GAT :members: :undoc-members: diff --git a/examples/gcn_mimic4.ipynb b/examples/gcn_mimic4.ipynb index e38ee941e..de0ff9e1c 100644 --- a/examples/gcn_mimic4.ipynb +++ b/examples/gcn_mimic4.ipynb @@ -6,7 +6,7 @@ "metadata": {}, "source": [ "# 1. Environment Setup\n", - "Use this section to configure deterministic behaviour and import the libraries required for the rest of the tutorial." + "Use this section to configure deterministic behaviour and import the libraries required for the rest of the tutorial.\n\n**Note:** earlier pyhealth versions had pyhealth.models.gnn call torch.manual_seed(3) / np.random.seed(1) at import time, so ``from pyhealth.models import GCN`` below would have silently overwritten the SEED = 42 set in this cell. This has been fixed: importing pyhealth.models no longer touches global RNG state, so the seeding above is respected." ] }, { @@ -139,7 +139,7 @@ "name": "stderr", "output_type": "stream", "text": [ - "Generating samples for MortalityPredictionMIMIC4 with 1 worker: 100%|██████████| 100/100 [00:00<00:00, 133.76it/s]" + "Generating samples for MortalityPredictionMIMIC4 with 1 worker: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 100/100 [00:00<00:00, 133.76it/s]" ] }, { @@ -154,7 +154,7 @@ "output_type": "stream", "text": [ "\n", - "Processing samples: 100%|██████████| 108/108 [00:00<00:00, 34584.27it/s]" + "Processing samples: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 108/108 [00:00<00:00, 34584.27it/s]" ] }, { @@ -450,7 +450,7 @@ "name": "stderr", "output_type": "stream", "text": [ - "Epoch 0 / 5: 100%|██████████| 3/3 [00:00<00:00, 5.11it/s]" + "Epoch 0 / 5: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 3/3 [00:00<00:00, 5.11it/s]" ] }, { @@ -466,7 +466,7 @@ "output_type": "stream", "text": [ "\n", - "Evaluation: 100%|██████████| 1/1 [00:00<00:00, 609.46it/s]" + "Evaluation: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 1/1 [00:00<00:00, 609.46it/s]" ] }, { @@ -485,7 +485,7 @@ "output_type": "stream", "text": [ "\n", - "Epoch 1 / 5: 100%|██████████| 3/3 [00:00<00:00, 250.36it/s]" + "Epoch 1 / 5: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 3/3 [00:00<00:00, 250.36it/s]" ] }, { @@ -501,7 +501,7 @@ "output_type": "stream", "text": [ "\n", - "Evaluation: 100%|██████████| 1/1 [00:00<00:00, 655.67it/s]" + "Evaluation: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 1/1 [00:00<00:00, 655.67it/s]" ] }, { @@ -519,7 +519,7 @@ "output_type": "stream", "text": [ "\n", - "Epoch 2 / 5: 100%|██████████| 3/3 [00:00<00:00, 285.02it/s]" + "Epoch 2 / 5: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 3/3 [00:00<00:00, 285.02it/s]" ] }, { @@ -535,7 +535,7 @@ "output_type": "stream", "text": [ "\n", - "Evaluation: 100%|██████████| 1/1 [00:00<00:00, 684.00it/s]" + "Evaluation: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 1/1 [00:00<00:00, 684.00it/s]" ] }, { @@ -553,7 +553,7 @@ "output_type": "stream", "text": [ "\n", - "Epoch 3 / 5: 100%|██████████| 3/3 [00:00<00:00, 285.06it/s]" + "Epoch 3 / 5: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 3/3 [00:00<00:00, 285.06it/s]" ] }, { @@ -569,7 +569,7 @@ "output_type": "stream", "text": [ "\n", - "Evaluation: 100%|██████████| 1/1 [00:00<00:00, 727.67it/s]" + "Evaluation: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 1/1 [00:00<00:00, 727.67it/s]" ] }, { @@ -587,7 +587,7 @@ "output_type": "stream", "text": [ "\n", - "Epoch 4 / 5: 100%|██████████| 3/3 [00:00<00:00, 332.15it/s]" + "Epoch 4 / 5: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 3/3 [00:00<00:00, 332.15it/s]" ] }, { @@ -603,7 +603,7 @@ "output_type": "stream", "text": [ "\n", - "Evaluation: 100%|██████████| 1/1 [00:00<00:00, 623.32it/s]" + "Evaluation: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 1/1 [00:00<00:00, 623.32it/s]" ] }, { @@ -651,7 +651,7 @@ "name": "stderr", "output_type": "stream", "text": [ - "Evaluation: 100%|██████████| 1/1 [00:00<00:00, 613.74it/s]\n" + "Evaluation: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 1/1 [00:00<00:00, 613.74it/s]\n" ] }, { @@ -665,7 +665,7 @@ "name": "stderr", "output_type": "stream", "text": [ - "Evaluation: 100%|██████████| 1/1 [00:00<00:00, 581.33it/s]" + "Evaluation: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 1/1 [00:00<00:00, 581.33it/s]" ] }, { @@ -713,7 +713,7 @@ "name": "stderr", "output_type": "stream", "text": [ - "Evaluation: 100%|██████████| 1/1 [00:00<00:00, 678.58it/s]" + "Evaluation: 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 1/1 [00:00<00:00, 678.58it/s]" ] }, { diff --git a/pyhealth/models/gnn.py b/pyhealth/models/gnn.py index 9ef323cb3..ae438841e 100644 --- a/pyhealth/models/gnn.py +++ b/pyhealth/models/gnn.py @@ -17,9 +17,6 @@ from pyhealth.processors import SequenceProcessor from pyhealth.models.embedding import EmbeddingModel -torch.manual_seed(3) -np.random.seed(1) - """Graph Neural Network models for PyHealth. This module provides implementations of Graph Convolutional Network (GCN) and diff --git a/tests/core/test_gnn.py b/tests/core/test_gnn.py index ef1468ca9..22e581ec7 100644 --- a/tests/core/test_gnn.py +++ b/tests/core/test_gnn.py @@ -1,5 +1,7 @@ """Unit tests for GNN models (GCN and GAT).""" +import subprocess +import sys import unittest import torch @@ -453,5 +455,52 @@ def test_custom_hyperparameters(self): self.assertIn("y_prob", ret) +class TestImportingModelsDoesNotResetGlobalRNG(unittest.TestCase): + """Regression test: pyhealth/models/gnn.py used to call + torch.manual_seed(3) / np.random.seed(1) at module level. Since + pyhealth/models/__init__.py does `from .gnn import GAT, GCN`, this ran + as a side effect of `import pyhealth.models` -- silently overwriting + any seed the user had already set for their own script, regardless of + whether they ever used GCN/GAT. This must run in a fresh subprocess: + by the time any in-process test executes, pyhealth.models is already + imported (and cached), so the side effect already happened once for + the whole test session and can't be observed from within it. + """ + + def test_import_does_not_consume_or_reset_rng_state(self): + script = ( + "import torch, numpy as np\n" + "torch.manual_seed(12345)\n" + "np.random.seed(54321)\n" + "expected_torch = torch.rand(3).tolist()\n" + "expected_np = np.random.rand(3).tolist()\n" + "torch.manual_seed(12345)\n" + "np.random.seed(54321)\n" + "import pyhealth.models\n" # the import under test + "actual_torch = torch.rand(3).tolist()\n" + "actual_np = np.random.rand(3).tolist()\n" + "assert actual_torch == expected_torch, (\n" + " f'importing pyhealth.models changed the torch RNG stream: '\n" + " f'{actual_torch} != {expected_torch}'\n" + ")\n" + "assert actual_np == expected_np, (\n" + " f'importing pyhealth.models changed the numpy RNG stream: '\n" + " f'{actual_np} != {expected_np}'\n" + ")\n" + "print('RNG_UNCHANGED')\n" + ) + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + timeout=120, + ) + self.assertEqual( + result.returncode, 0, + f"subprocess failed:\nstdout={result.stdout}\nstderr={result.stderr}", + ) + self.assertIn("RNG_UNCHANGED", result.stdout) + + if __name__ == "__main__": unittest.main()