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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Histogram blocks refactored #25

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

patlevin
Copy link

Histogram Block Refactoring

Motivation

When using histogram loss, loss calculation can take up a significant portion of the training process.
This PR tries to improve on that by refactoring the original implementation of the losses while staying
consistent with the output of the original implementations.

Content

This PR contains refactored histogram losses. It adds a common base class to make future additions simpler as well as to
reduce code duplication. Common functionality, such as the kernel methods for resizing (sampling), pixel counting, and intensity scaling have been extracted into functions found in the new base class module.

Results

The refactored histogram blocks are now significantly smaller and easier to understand (at least in my opinion馃).
The public interface and default parameters have been kept as-is, so no changes should be required on the caller side.
Performance tests showed an improvement of >5x for each RGB-uv, rg-chroma, and Lab blocks using their respective default parameters.

This performance improvement only increases on non-datacentre class hardware, such as consumer and professional GPUs.
The performance uplift on consumer grade hardware has been observed to be up to 20x due to hardware limitations of these devices (primarily due to the very limited double precision support).

Accuracy w.r.t. to the original implementation is acceptable. In all but some synthetic tests, mean deviation between original and refactored versions are well within the machine epsilon of 32-bit floating point maths.

Limitations

The refactored version drops support for input images bigger than about 2.1 gigapixels (i.e. roughly 45,000 by 45,000 pixels).
I personally don't think this limitation will pose an issue in the foreseeable future, but I think it's worth mentioning nonetheless馃槈.

And finally: thank you for your great work and for publishing your code!

@mahmoudnafifi
Copy link
Owner

Hi @patlevin,

Thank you so much for your contribution and all the effort in testing and explaination. I am quite busy those two weeks and I will do my best to check the PR soon.

Thanks again!

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.

None yet

2 participants