-
Notifications
You must be signed in to change notification settings - Fork 112
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
[oneDPL] Histogram APIs #546
Closed
danhoeflinger
wants to merge
27
commits into
uxlfoundation:main
from
danhoeflinger:dev/dhoeflin/oneDPL_histogram
Closed
Changes from 23 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
4a5dd2d
first draft histogram spec
danhoeflinger 477f203
more explicit language / code
danhoeflinger 82032db
simplifying and combining overloads
danhoeflinger fb2cfc8
formatting
danhoeflinger 7c97cf5
removing extra space
danhoeflinger f25a685
formatting
danhoeflinger add3a6c
semicolon
danhoeflinger d17d582
minor wording change
danhoeflinger e790069
wording
danhoeflinger 98e1628
update language in introduction
danhoeflinger 0175f31
removing random access iterator explicit req
danhoeflinger 11d0ae5
fixing overflow of bins requirement
danhoeflinger 16aec2c
improving the language on types, sortedness, and definition of mapping
danhoeflinger f14756a
using `operator<=`
danhoeflinger 10e5377
renaming boundary variables for clarity
danhoeflinger 5b099f3
formatting
danhoeflinger 1a91065
removing explicit requirement for output data allocation
danhoeflinger 5fcec63
adding information about the number of bins for each overload
danhoeflinger e0c54bc
num_bins -> num_intervals
danhoeflinger 281adca
Apply suggestions from code review
danhoeflinger e463bf9
applying suggestion
danhoeflinger df2a94b
Adding return value description
danhoeflinger 91d46fd
conforming to conventions for :code:`foo` vs ``foo``
danhoeflinger 529130c
forcing interval begin and end to be the value type of the input sequ…
danhoeflinger cb50f42
Update source/elements/oneDPL/source/parallel_api.rst
danhoeflinger 04e62ff
align numbers for visibility
danhoeflinger 5d01d29
Avoid ambiguity for the number of bins
akukanov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
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.
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.
I suggest swapping the order of OutputIt and ValueType to be consistent with their use in the API.
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.
I believe you have caught something here that is a problem.
I have a proposal to fix it, but I think we want to do more than just reorder the template parameters.
Instead of having a free floating
ValueType
template parameter. I believe we should instead directly usestd::iterator_traits<InputIt>::value_type
as the type offirst_interval_begin
andlast_interval_end
.I think it is better for force users to provide bounds which are convertible to the
value_type
of the input sequence, and do the conversion once when accepting the arguments rather than each time in the comparison operator. This will change the signature and some of the language. I have updated it accordingly, please take a look.Also, while this is not the same exact API as what is currently implemented in oneDPL, it is not incompatible with the existing implementation's API. I believe we can continue to support that API as an extension without a breaking change, but lets discuss that in the implementation PR, which will be up shortly.