Incorrect ref version for pooling with count_include_pad=True
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 333
- Forks
- 150
- Avg merge
- 4d 19h
- Merged PRs (30d)
- 54
Description
The following fails verification:
p = migraphx.program()
m = p.get_main_module()
p_x = m.add_parameter("x", migraphx.shape(type="float_type", lens=[1, 1, 2]))
x_1 = m.add_instruction(migraphx.op("pooling", padding=[1,2], stride=[1], lengths=[3], dilations=[1], count_include_pad=True), [p_x]) # migraphx.shape(type="float_type", lens=[1, 1, 3])
m.add_return([x_1])
However, the gpu is correct in this case, but the ref version is not. The ref version changes the size of the window in certain cases which produces an incorrect value as count_include_pad=True should always use the same window size. These changes were made in #1823, but are wrong. There is no reason to adjust the window size as we already check for out of bounds and skip.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Reproduce the failure with the Python MIGraphX pooling example from the issue, then compare the reference and GPU results when count_include_pad=True. Trace the reference pooling implementation and the changes from #1823; done means both versions use the same window size and verification passes for the reported case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- backend, machine-learning
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100