Drew's attempt at reproducing Hurum's work#18
Draft
drewoldag wants to merge 2 commits into
Draft
Conversation
…e amount of memory, need to move normalization into the model. Able to train 1 epoch, loss curve is suspicious.
Contributor
Author
|
The output from |
There was a problem hiding this comment.
Pull request overview
This PR is a WIP attempt to reproduce Hurum’s KBMOD ML work using the Hyrax framework by migrating existing Fibad-based components, adding a Hurum-inspired network, and updating dataset/config/notebook wiring for Hyrax-driven training.
Changes:
- Migrates model/dataset registration from Fibad to Hyrax and updates README/packaging references accordingly.
- Adds a new
KBModNet(Hurum-style) model and a new Hyrax-compatibleKbmodStampsdataset implementation (including optional augmentation and metadata table creation). - Adds an exploratory notebook (
drews_attempt.ipynb) and updates the docs training notebook toward Hyrax usage.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| user_config.toml | Removes the local user config file (notebook/docs still reference it). |
| src/kbmod_ml/models/resnet.py | Switches to Hyrax registry and changes model to ResNet18; updates training target handling. |
| src/kbmod_ml/models/hurum_net.py | Adds Hurum-inspired KBModNet model with focal loss and augmentation. |
| src/kbmod_ml/models/cnn.py | Switches CNN registration from Fibad to Hyrax. |
| src/kbmod_ml/default_config.toml | Simplifies kbmod-ml config and adds augment flag. |
| src/kbmod_ml/data_sets/kbmod_stamps.py | Reworks KbmodStamps into a HyraxDataset-compatible dataset with augmentation and metadata. |
| scripts/true_positive_stamp_generation.py | Minor formatting/import ordering adjustments. |
| scripts/random_selections.py | Minor import ordering/whitespace cleanup. |
| scripts/false_positive_stamp_generation.py | Minor import ordering cleanup. |
| README.md | Updates dependency instructions from Fibad to Hyrax. |
| pyproject.toml | Updates dependency comment from Fibad to Hyrax. |
| drews_attempt.ipynb | Adds an exploratory Hyrax training notebook (currently environment/path-specific with outputs). |
| docs/notebooks/train_model.ipynb | Updates notebook toward Hyrax but contains several now-broken references. |
Comments suppressed due to low confidence (3)
src/kbmod_ml/data_sets/kbmod_stamps.py:52
super().__init__is called twice (once with onlyconfig, and again withconfig, metadata_table). This is redundant at best and can overwrite HyraxDataset state at worst. Initialize the base class once after you’ve built the metadata table (or only once with the correct arguments).
def __init__(self, config, data_location=None):
super().__init__(config)
# data_dir = config["general"]["data_dir"]
true_positive_file_name = config["kbmod_ml"]["true_positive_file_name"]
false_positive_file_name = config["kbmod_ml"]["false_positive_file_name"]
true_data_path = os.path.join(data_location, true_positive_file_name)
false_data_path = os.path.join(data_location, false_positive_file_name)
if not os.path.isfile(true_data_path):
raise ValueError(f"Could not find {true_positive_file_name} in provided {data_location}")
if not os.path.isfile(false_data_path):
raise ValueError(f"could not find {false_positive_file_name} in provided {data_location}")
true_positive_samples = np.load(true_data_path)
logger.warning(f"Loaded {len(true_positive_samples)} true positive samples from {true_data_path}")
false_positive_samples = np.load(false_data_path)
logger.warning(f"Loaded {len(false_positive_samples)} false positive samples from {false_data_path}")
n_tp = len(true_positive_samples)
n_fp = len(false_positive_samples)
raw_data = np.concatenate([true_positive_samples, false_positive_samples[:, 0:3]])
raw_labels = np.concatenate([
np.ones(n_tp, dtype=np.int64),
np.zeros(n_fp, dtype=np.int64),
])
self._data = raw_data.astype(np.float32)
# self._data = self._normalize(raw_data)
self._labels = raw_labels
seed = config["data_set"]["seed"] if config["data_set"]["seed"] else 42
# self._arrange_for_hyrax_splits(n_tp, n_fp, seed)
self.augment = config["kbmod_ml"]["augment"]
metadata_table = self._read_metadata()
super().__init__(config, metadata_table)
src/kbmod_ml/data_sets/kbmod_stamps.py:204
_read_metadata()storesobject_idas integer indices (np.arange(...)), butget_object_id()returns a zero-padded string. If Hyrax uses the metadata table forprimary_id_field='object_id', this type/format mismatch can break joins/lookups. Storeobject_idconsistently (e.g., padded strings everywhere).
def ids(self):
return np.arange(len(self._data))
def shape(self):
width, height = self._data[0][0].shape
return (3, width, height)
def get_object_id(self, idx):
# Return a 0-padded string of the index with the number of digits needed
# for the largest index, e.g. "000123" for index 123 if there are less
# than 1 million samples
num_digits = len(str(len(self._data) - 1))
return f"{idx:0{num_digits}d}"
def get_classification(self, idx):
return self._labels[idx]
def get_stamps(self, idx):
x = self._data[idx]
if self.augment:
# reproduce this code using numpy instead of torch
x = np.rot90(x, k=np.random.randint(0, 4), axes=(1, 2))
if np.random.rand() > 0.5: x = np.flip(x, axis=2)
if np.random.rand() > 0.5: x = np.flip(x, axis=1)
x = x + np.random.randn(*x.shape).astype(np.float32) * 0.05
return x
def _read_metadata(self):
from astropy.table import Table
return Table({
"object_id": self.ids(),
"classification": self._labels,
})
src/kbmod_ml/data_sets/kbmod_stamps.py:20
data_locationdefaults toNone, but the constructor immediately callsos.path.join(data_location, ...), which will raise aTypeErrorwith an unclear message. Ifdata_locationis required, validate it early and raise a clearValueErrorwhen it’s missing.
def __init__(self, config, data_location=None):
super().__init__(config)
# data_dir = config["general"]["data_dir"]
true_positive_file_name = config["kbmod_ml"]["true_positive_file_name"]
false_positive_file_name = config["kbmod_ml"]["false_positive_file_name"]
true_data_path = os.path.join(data_location, true_positive_file_name)
false_data_path = os.path.join(data_location, false_positive_file_name)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+30
to
+41
| "data_request = {\n", | ||
| " \"train\": {\n", | ||
| " \"data\": {\n", | ||
| " \"dataset_class\": \"kbmod_ml.data_sets.kbmod_stamps.KbmodStamps\",\n", | ||
| " \"data_location\": \"/home/drew/data/kbmod_ml/\",\n", | ||
| " \"primary_id_field\": \"object_id\",\n", | ||
| " \"split_fraction\": 0.70,\n", | ||
| " \"dataset_config\": {\n", | ||
| " \"kbmod_ml\": {\n", | ||
| " \"augment\": True,\n", | ||
| " \"true_positive_file_name\": \"tp_combined_5_7sigma.npy\",\n", | ||
| " \"false_positive_file_name\": \"fp_combined_5_7sigma.npy\",\n", |
…he different files as inputs.
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.
WIP - Attempting to reproduce Hurum's work in Hyrax
Notes
13 May
I'm using the two data files that are available on epyc here:
Copied to local drive with scp.
I modified the KbmodStamps dataset class to not do normalization just to avoid the startup penalty.
Suggest that we move the normalization to the
forwardmethod to ensure that all data that goes in is properly normalized.Note for Drew for later
Running h.train(trace=2) indicated that DataProvider.resolve_data took 0.2ms and collate took 0.1ms. These seem like very large numbers to retrieve only 2 samples. However, the runtime for get_stamps was 0.12ms and 0.28ms for the two samples.
I ran only 1 epoch (took about 14 minutes on my local machine) and noticed the telltail sign that we've been seeing in the loss curve of an immediate drop and then consistent noise, i.e. not learning anything after the first few batches.
Tomorrow: