Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ground truths only data loader #93

Merged
merged 3 commits into from
May 2, 2024

Conversation

fsherry
Copy link
Member

@fsherry fsherry commented May 2, 2024

This adds a CTBenchmarkingExperiment, called GroundTruthCT, and a corresponding task 'groundtruth', which only loads the ground truth reconstructions, for example for use in training a denoiser. The time saved processing the sinograms (which happens on CPU) allows for higher GPU utilisation.

The issue of time wasted on preprocessing sinograms when only training denoisers was mentioned in #92.

@fsherry fsherry force-pushed the groundtruth_dataloader branch from dab52f1 to 87a962c Compare May 2, 2024 00:42
Copy link
Member

@AnderBiguri AnderBiguri left a comment

Choose a reason for hiding this comment

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

Happy to to these changes myself, just noting them so I don't forget :D

LION/data_loaders/deteCT.py Outdated Show resolved Hide resolved
LION/data_loaders/deteCT.py Show resolved Hide resolved
@fsherry fsherry force-pushed the groundtruth_dataloader branch from b346d97 to e97a008 Compare May 2, 2024 13:34
@fsherry
Copy link
Member Author

fsherry commented May 2, 2024

@AnderBiguri Please check that it was right to add this, but it seemed like there was a missing else in the branching on self.do_recon for target (which seems to be ok for input). As a result, it would compute a reconstruction, but then still load the reconstruction from file, overwriting the computed reconstruction. The relevant snippet is here, it looks like there should be an else before line 419, which I added in commit e97a008:

# if target is reconstruction, we need to load the reconstruction
elif self.task in ["recon2recon", "sino2recon", "groundtruth"]:
if self.do_recon:
sinogram = self.__load_and_preprocess_sinogram__(
index, self.target_mode
)
op = deteCT.get_operator()
if self.recon_algo == "nag_ls":
target = nag_ls(op, sinogram, 100, min_constraint=0)
elif self.recon_algo == "fdk":
target = fdk(op, sinogram)
target = self.__load_and_preprocess_reconstruction__(
index, self.target_mode
)

@fsherry fsherry requested a review from AnderBiguri May 2, 2024 14:25
@AnderBiguri
Copy link
Member

@fsherry correct, good catch!

@AnderBiguri AnderBiguri merged commit 850fffd into CambridgeCIA:main May 2, 2024
1 check passed
@fsherry fsherry deleted the groundtruth_dataloader branch May 2, 2024 17:36
@fsherry fsherry mentioned this pull request Jul 16, 2024
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