-
-
Notifications
You must be signed in to change notification settings - Fork 502
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
Fix halo implementation and tiling artefact #113
Conversation
Previously each patch is mirror-padded with itself
Only changed `predictor.py` manually
This is great @qin-yu! Thanks a lot for the PR. It looks good to me from the first glance. I'll do a proper review tomorrow, merge it and release the new version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, I've left couple of comments/questions
Here I illustrate why raw is padded on all sides but slicing is simply extending the end by 2 x halo self.raw_padded = mirror_pad(self.raw, self.halo_shape)
raw_idx_padded = tuple(slice(this_index.start, this_index.stop + 2 * this_halo, None) for this_index, this_halo in zip(raw_idx, self.halo_shape)) On the left I show how padded input are predicted and put back to the original shape; on the right I show how patches should be sliced correctly: Note that the image in #113 (comment) is produced with patch shape = stride shape, i.e. the tiling can only match if slicing is done properly. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the PR
Fix halo implementation and tiling artefact
I've revisited the issue of the tiling artifact and identified that the current implementation of the halo is incorrect. For each patch, the halo should constitute the surrounding margins of that patch. The use of mirror padding is responsible for our tiling artifact.
Before my fix, with a patch size of
96x96x96
, a stride of96x96x96
, and a halo of32x64x64
, the prediction exhibited a clear tiling artifact. With the correct implementation of the halo, using the same configuration, the prediction shows significant improvement. Note that with such settings, there is no overlap between neighbour patches, yet we still see almost no artefact with the new implementation.Comparison
Left: Mirror pad; Right: Halo pad
Design Choices
Dataset
's get method, while removal of padding happens inPredictor
. SincePredictor
s takes dataDataset
s wrapped in loaders, I move halo config toSliceBuilder
under loader config.Predictor
s can access the halo directly from the inputDataset
s.Old and New Config Files
Old config file for inference (Predictor taking halo, patch mirror-padded)
New config file for inference (SliceBuilder taking halo, patch halo-padded)
Required changes in config files: