Skip to content

Add CheXlocalize_Dataset for CheXpert official val/test + segmentation masks - #191

Open
AmlanMishra2004 wants to merge 11 commits into
mlmed:mainfrom
AmlanMishra2004:add-chexlocalize-dataset
Open

Add CheXlocalize_Dataset for CheXpert official val/test + segmentation masks#191
AmlanMishra2004 wants to merge 11 commits into
mlmed:mainfrom
AmlanMishra2004:add-chexlocalize-dataset

Conversation

@AmlanMishra2004

Copy link
Copy Markdown
Contributor

Summary

Adds CheXlocalize_Dataset, a loader for CheXlocalize (Saporta et al., Nature Machine Intelligence 2022, https://doi.org/10.1038/s42256-022-00536-x): the official CheXpert validation/test images and labels, plus radiologist ground-truth segmentation masks for 10 pathologies.

CheX_Dataset can't load this data as-is:

  • It assumes Sex/Age/Frontal-Lateral/AP-PA columns exist. CheXlocalize's blinded official test_labels.csv omits them to prevent re-identification of the test set — loading it with CheX_Dataset raises KeyError: 'Frontal/Lateral'.
  • Even with those columns present, it infers the train/valid split by string-matching 'train'/'valid' in the CSV path, and raises a bare NotImplementedError otherwise.

CheXlocalize_Dataset:

  • Synthesizes safe defaults for the missing demographic/view columns instead of assuming they exist.
  • Extracts patient IDs and views without depending on the split name.
  • Normalizes CheXlocalize's path quirks (strips CheXpert-v1.0/ prefix; the val CSV's valid/ prefix vs. the val/ directory name on disk).
  • Adds pathology_masks=True + segmentation_jsonpath=... support for CheXlocalize's ground-truth segmentation JSON (COCO RLE via pycocotools), following the same pattern as SIIM_Pneumothorax_Dataset.get_pathology_mask_dict.

Testing

  • 3 new tests in tests/test_dataloaders.py covering: loading the blinded test CSV (missing columns + no train/valid string match), valid/val/ path normalization, and segmentation mask decoding (including that pathologies absent from the mask JSON correctly yield an all-zero mask).
  • Full tests/test_dataloaders.py suite passes (17/17).
  • Verified against a real CheXlocalize download: test split loads 500 samples (668 raw rows deduped to the documented 500 unique patients), val split loads 200 samples (234 CXRs deduped to the documented 200 unique patients) with correctly decoded masks — e.g. a known Cardiomegaly + Enlarged Cardiomediastinum positive patient shows nonzero mask pixels only for those two pathologies, zero everywhere else.
  • Confirmed CheX_Dataset does in fact fail on the real test_labels.csv with KeyError: 'Frontal/Lateral'.
  • pep8.sh clean on the new code.

…n masks

CheX_Dataset can't load CheXlocalize's data: it infers the train/valid
split by string-matching the CSV path and raises NotImplementedError
otherwise, and it assumes Sex/Age/Frontal-Lateral/AP-PA columns exist,
which the blinded official test_labels.csv omits to prevent
re-identification.

CheXlocalize_Dataset handles both, and adds pathology_masks support for
the 10 pathologies with radiologist ground-truth segmentations (COCO
RLE via pycocotools), following the pattern in SIIM_Pneumothorax_Dataset.
@AmlanMishra2004

Copy link
Copy Markdown
Contributor Author

Opened the pre-PR issue per CONTRIBUTING.md: #192

@ieee8023 ieee8023 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! I added some initial comments. The main one is that I'd like all the dataloaders to load masks in the same format. Can you look into aligning this dataloader with that patrern? Look at objectcxr, vinbrain, nih, they all have the same pathology masks keys for each sample.

Also if you can add the license of the annotations into the docstring.

if "Frontal/Lateral" not in self.csv.columns:
self.csv["Frontal/Lateral"] = np.where(
self.csv["Path"].str.contains("_lateral", case=False), "Lateral", "Frontal")
if "AP/PA" not in self.csv.columns:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this assumption in the dataset documentation? Otherwise setting "Unknown" is probably better.

``gt_segmentations_test.json``. Masks are stored as COCO RLE
(``pycocotools``) keyed by CXR id (``patientX_studyY_viewZ_frontal``).
Requires ``pycocotools`` to decode.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an example creating the dataset object, with the names of the files from the dataset set as the correct arguments. No need for paths. Just like csvpath="specialfoldername.xml"

Cardiomediastinum, Fracture, Lung Lesion, Lung Opacity, Pleural Other,
Pneumonia, Pneumothorax, Support Devices.

**Segmentation masks** are available for 10 of these pathologies (all

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create a notebook do demonstrate this dataloader? The interface should be the same as the other dataloaders with pathology masks. Can you make the format of the masks the same. Can you transform the segmentations into the same format? I think there is already code to transform from the coco format in another dataloader si we can avoid the pycocotools dependency.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dug into this — couldn't find an existing COCO RLE mask decoder anywhere in the repo (checked NIH/VinBrain/ObjectCXR/COVID19/SIIM). SIIM's rle2mask decodes a different, incompatible format (flat start length pairs, not COCO's compressed binary counts string).

The closest match is TBX11K_Dataset, which does load a real COCO-schema JSON (images/annotations/categories), but it only reads bbox fields — it never touches compressed RLE pixel masks.

I did try writing a pure-Python decoder for COCO's compressed RLE (translated from maskApi.c), but found it produces wrong masks whenever the annotated region touches pixel (0,0) — a real, not-hypothetical case in this data — so I backed that out. Given pycocotools is only lazily imported when masks are actually requested, I'd rather keep it than risk a hand-rolled RLE codec. Happy to revisit if you can point me at the code you had in mind.

Comment thread torchxrayvision/datasets.py Outdated
# the blinded test set is AP, PA, or a mix — leave it unknown
# rather than guess. `views` defaults to include "UNKNOWN" so
# these frontal images aren't silently dropped.
self.csv["AP/PA"] = "UNKNOWN"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make the strings match other fields like this

Suggested change
self.csv["AP/PA"] = "UNKNOWN"
self.csv["AP/PA"] = "Unknown"

Comment thread torchxrayvision/datasets.py Outdated
data_aug=None,
seed=0,
unique_patients=True,
pathology_masks=False,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this dataset is for pathology masks let's default this to true so the user doesn't need to figure it out.

Suggested change
pathology_masks=False,
pathology_masks=True,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment to run the notebook and include the images but that may be an issue with the license so disregard it.

@AmlanMishra2004

Copy link
Copy Markdown
Contributor Author

Pushed fixes for the latest review round:

  • AP/PA unknown sentinel now "Unknown" (matches Sex), views default updated to match
  • pathology_masks now defaults to True

Notebook comment noted as disregarded (license). Let me know if there's more.

@AmlanMishra2004

Copy link
Copy Markdown
Contributor Author

Summary of everything addressed since the last review round:

  • Mask format: get_mask_dict returns a sparse {pathology_index: mask} dict, matching NIH_Dataset/VinBrain_Dataset/ObjectCXR_Dataset (no zero-masks for absent labels).
  • License: added to the class docstring (Stanford CheXlocalize Research Use Agreement, cheXlocalize benchmarking code separately MIT-licensed).
  • Usage example: added to the docstring with real CheXlocalize filenames (csvpath="test_labels.csv", etc.).
  • AP/PA sentinel / pathology_masks default: already covered in my earlier comment.
  • Test suite: CI was failing after the "Unknown" casing and pathology_masks=True changes, so I updated the two affected tests to match (one assertion, and pathology_masks=False on two tests that don't exercise masks, so they don't need pycocotools). All 3 CI platforms now pass.

Still open: whether to keep pycocotools for RLE decoding (see my reply above). Let me know if you'd like me to revisit that.

@ieee8023

Copy link
Copy Markdown
Member

Everything looks good so far. I'm having issues downloading the cheXlocalize dataset. Once I can do that and test the code I'll approve it.

@AmlanMishra2004

Copy link
Copy Markdown
Contributor Author

Just checking in, let me know if there's anything else you need from me, or if I can help with the CheXlocalize download in any way.

@ieee8023 ieee8023 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it out and ran into one issue, lets remove than line and then if you can update the docstring to include counts. Then I think it is all set.

Cardiomediastinum, Fracture, Lung Lesion, Lung Opacity, Pleural Other,
Pneumonia, Pneumothorax, Support Devices.

**Segmentation masks** are available for 10 of these pathologies (all

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have the counts of what masks are available per pathology in both the test and valid sets in this docstring.

# patient/view parsing below works regardless of the val/test split
self.csv["Path"] = self.csv["Path"].str.replace("CheXpert-v1.0-small/", "", regex=False)
self.csv["Path"] = self.csv["Path"].str.replace("CheXpert-v1.0/", "", regex=False)
self.csv["Path"] = self.csv["Path"].str.replace(r"^valid/", "val/", regex=True)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This created an issue when I tried to load the validation set and I needed to comment out this line. Lets remove it.

Suggested change
self.csv["Path"] = self.csv["Path"].str.replace(r"^valid/", "val/", regex=True)

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.

2 participants