X Tutup
Skip to content

Join does not always respect the order of provided parameters (#3511)#3513

Merged
christophe-murphy merged 2 commits intoarrayfire:masterfrom
willyborn:issue3511
Feb 26, 2025
Merged

Join does not always respect the order of provided parameters (#3511)#3513
christophe-murphy merged 2 commits intoarrayfire:masterfrom
willyborn:issue3511

Conversation

@willyborn
Copy link
Contributor

@willyborn willyborn commented Oct 15, 2023 •
edited
Loading

Arrays were be joined in the order the JIT engine generated the arrays iso the order of the parameters.

Description

  1. A wrong assumption was made that the JIT engine would generate the arrays in the same order as provided by the parameters.
    This assumption is violated when one of the generated arrays is an intermediate result of a previous specified JIT generated array.
    Example of error condition:
af::array A{af::constant(1.,10)};
af::array R{af::join(0,-A,A)};  --> generated af::join(0,A,-A) because A is an intermediate result of -A.

In this fix, the order of the parameters is imposed, independent from the order of JIT generation.
PS: This bug appears in OPENCL and CUDA.

  1. The same 'unique' identifier "funcName" was generated for JIT kernels, with outputs as only difference.
    As result, a different kernel could be executed than intended, dependent on the order of the JIT kernel generation.

Additional information about the PR answering following questions:

  • bug fix
  • can be back ported
  • extra test is added covering a large set of possible combinations of similar outputs (567 combinations)

Fixes: #3511

Changes to Users

None

Checklist

  • Rebased on latest master
  • Code compiles
  • Tests pass
  • Functions added to unified API
  • Functions documented

@willyborn
Copy link
Contributor Author

willyborn commented Oct 24, 2023

Do not merge yet.
I discovered a new combination giving unexpected results.
The testing case will be expended to include more logical combinations (JIT nodes, Block nodes, repeating, dependencies, ...)
You will get an updated PR in the coming days.

@willyborn
Copy link
Contributor Author

willyborn commented Oct 26, 2023

Code is updated and join tests are expanded with the generation of multiple join combinations.

For some joins, the kernel of a previous join could be used since both generated the same 'unique' funcName (while having a different kernel) resulting in the reuse of the previously cached kernel iso generating the new one.
The chance of happening this is very rare, explaining why I added a systematic combination generator to the tests.

@willyborn willyborn changed the title Join does not always respect the order of provided parameters (#3511) Join does not always respect the order of provided parameters (#3511 & #3532) Feb 13, 2024
@willyborn willyborn mentioned this pull request Feb 13, 2024
Closed
@willyborn willyborn changed the title Join does not always respect the order of provided parameters (#3511 & #3532) Join does not always respect the order of provided parameters (#3511) Feb 20, 2024
@umar456
Copy link
Member

umar456 commented Mar 7, 2024

Updated the getFuncName parameters in oneapi.

@melonakos melonakos added this to the 3.10 milestone Feb 5, 2025
@christophe-murphy christophe-murphy self-requested a review February 21, 2025 00:47
@christophe-murphy christophe-murphy merged commit 48b7a9e into arrayfire:master Feb 26, 2025
2 of 4 checks passed
willyborn added a commit to willyborn/arrayfire that referenced this pull request Jun 14, 2025
@willyborn
Join does not always respect the order of provided parameters (oneapi) (
811669f 
arrayfire#3511)(arrayfire#3513)
@willyborn willyborn mentioned this pull request Jun 14, 2025
Merged
5 tasks
christophe-murphy pushed a commit that referenced this pull request Jun 25, 2025
@willyborn
Join does not always respect the order of provided parameters (oneapi) (
700db10 
#3511)(#3513) (#3667)

* Adds test helpers for temporary array formats (JIT, SUB, ...)

* Join does not always respect the order of provided parameters (oneapi) (#3511)(#3513)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@christophe-murphy christophe-murphy christophe-murphy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.10

Development

Successfully merging this pull request may close these issues.

[BUG] found on AF 3.8.3 and 3.9.0 for OPENCL, possibly with af::join

4 participants

@willyborn @umar456 @christophe-murphy @melonakos
X Tutup