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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support where-clause evaluation in registration annotations #3841

Conversation

jeremiah-corrado
Copy link
Contributor

@jeremiah-corrado jeremiah-corrado commented Oct 15, 2024

Add (primitive) support for where-clause evaluation to registerCommand and instantiateAndRegister. Both annotations now evaluate the procedure's where-clause and will only generate instantiations for sets of generic parameters where the where-clause would evaluate to true.

The primary goal of this change is to be able to remove extraneous overloads of command procedures that simply return an error message indicating that some set of types and/or array ranks are not supported for that command. For example, in the following code snippet, the second overload of asdf can now be removed because the build system won't attempt to generate an instantiation for it:

@arkouda.registerCommand
proc asdf(x: [?d] ?t): [d] t throws
  where t != bigint
{
  // ...
}

proc asdf(x: [?d] ?t): [d] t throws
  where t == bigint
    do throw new Error("'asdf' does not support 'bigint'");

This feature is not totally general-purpose yet. Current limitations:

  • only some param-function calls are currently supported. Specifically, support forisIntegralType and other related procedures from Chapel's Types module is hard-coded in, but any other function calls within a where-clause will not work
  • references to queried array types and queried domains are supported; however:
    • only the rank of a queried domain can be referenced (as in the code example below), none of the domain's other param or type methods/fields are accessible
    • referencing an array's etype or rank directly is not yet supported (ex: where x.etype != bool && x.rank < 3)
  • referencing param or type fields of a non-array argument is not yet supported (ex: where mySparseSym.matLayout == Layout.CSR)

Eventually, registerCommands.py should be modified to use the Compiler's resolver to speculatively evaluate where-clauses in a much more robust manner. This would in principle remove the above limitations, but is likely a more significant development effort than the solution in this PR.

Limitations aside, this PR allows most of the extraneous procedures currently in Arkouda to be removed.

Note: one can opt out of this feature by setting the ignoreWhereClause argument in the annotation call. This could be useful in an example like the one below, or to get around one of the limitations listed above.

@arkouda.registerCommand(ignoreWhereClause=true)
proc jkl(x: [?d] ?t): [d] t throws
  where d.rank == 1 {
    // special 1D implementation ... 
}

proc jkl(x: [?d] ?t): [d] t throws
  where d.rank > 1 {
    // general ND implementation ... 
}

(resolves: #3837)

@jeremiah-corrado jeremiah-corrado marked this pull request as ready for review October 15, 2024 22:27
Signed-off-by: Jeremiah Corrado <[email protected]>
Signed-off-by: Jeremiah Corrado <[email protected]>
@stress-tess stress-tess requested review from ajpotts, stress-tess and bmcdonald3 and removed request for ajpotts and stress-tess October 16, 2024 17:33
Copy link
Contributor

@ajpotts ajpotts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Look forward to using this!

Copy link
Member

@stress-tess stress-tess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this jeremiah!

src/registry/register_commands.py Show resolved Hide resolved
@stress-tess stress-tess added this pull request to the merge queue Oct 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2024
@jeremiah-corrado jeremiah-corrado added this pull request to the merge queue Oct 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2024
@stress-tess stress-tess added this pull request to the merge queue Oct 17, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 17, 2024
* add primitive where-clause evaluation (for instantiateAndRegister) to register_commands.py

Signed-off-by: Jeremiah Corrado <[email protected]>

* remove uninstantiated overloads from commands annotated with instantiateAndRegister

Signed-off-by: Jeremiah Corrado <[email protected]>

* remove match statements from register_commands.py

Signed-off-by: Jeremiah Corrado <[email protected]>

* add error message for 'pad' w/ bigint

Signed-off-by: Jeremiah Corrado <[email protected]>

* add preliminary support for where-clause evaluation in 'registerCommand'

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix misinterpretation of explicit type widths (e.g., uint(64)) in where clauses

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix error message

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix error message

Signed-off-by: Jeremiah Corrado <[email protected]>

---------

Signed-off-by: Jeremiah Corrado <[email protected]>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2024
…d in ReductionMsg. Fix 'axis' argument in 'ak.sum'

Signed-off-by: Jeremiah Corrado <[email protected]>
Signed-off-by: Jeremiah Corrado <[email protected]>
@jeremiah-corrado jeremiah-corrado added this pull request to the merge queue Oct 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 21, 2024
@stress-tess stress-tess added this pull request to the merge queue Oct 21, 2024
Merged via the queue into Bears-R-Us:master with commit 76408db Oct 21, 2024
11 checks passed
@jeremiah-corrado jeremiah-corrado deleted the annotation-where-clause-evaluation branch October 21, 2024 13:39
drculhane pushed a commit to drculhane/arkouda that referenced this pull request Oct 23, 2024
…Us#3841)

* add primitive where-clause evaluation (for instantiateAndRegister) to register_commands.py

Signed-off-by: Jeremiah Corrado <[email protected]>

* remove uninstantiated overloads from commands annotated with instantiateAndRegister

Signed-off-by: Jeremiah Corrado <[email protected]>

* remove match statements from register_commands.py

Signed-off-by: Jeremiah Corrado <[email protected]>

* add error message for 'pad' w/ bigint

Signed-off-by: Jeremiah Corrado <[email protected]>

* add preliminary support for where-clause evaluation in 'registerCommand'

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix misinterpretation of explicit type widths (e.g., uint(64)) in where clauses

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix error message

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix error message

Signed-off-by: Jeremiah Corrado <[email protected]>

* refactor 'sum' to not rely on where-clauses for dispatching

Signed-off-by: Jeremiah Corrado <[email protected]>

* refactor prod, max, and min to take their 'axis' argument as a list

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix mypy errors and multi-dim build failure

Signed-off-by: Jeremiah Corrado <[email protected]>

* create list-specific implementations of aryUtil helper procedures used in ReductionMsg. Fix 'axis' argument in 'ak.sum'

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix return type change to reduction procedures

Signed-off-by: Jeremiah Corrado <[email protected]>

* fix reducedShape helper for lists

Signed-off-by: Jeremiah Corrado <[email protected]>

---------

Signed-off-by: Jeremiah Corrado <[email protected]>
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.

argTypeReductionMessage axis argument as list
4 participants