Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions loopy/target/c/__init__.py
Copy link
Owner

Choose a reason for hiding this comment

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

Would it make sense to test this? (Not saying we should, just raising the possibility.)

Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,36 @@
yield ("08_c_math", "#include <math.h>")


class GSLCallable(ScalarCallable):
@override
def with_types(self,
arg_id_to_dtype: Mapping[int | str, LoopyType],
clbl_inf_ctx: CallablesInferenceContext,
) -> tuple[Self, CallablesInferenceContext]:
name = self.name

for id in arg_id_to_dtype:
if not isinstance(id, int):
raise LoopyError(f"'{name}' can take only positional arguments")
Comment on lines +857 to +859
Copy link
Owner

Choose a reason for hiding this comment

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

If you do this as a list comprehension, you may not have to cast below.


arg_num_to_dtype = {cast("int", id): t for id, t in arg_id_to_dtype.items()}
arg_num_to_dtype[-1] = arg_num_to_dtype[max(arg_num_to_dtype)]

name_in_target = f"gsl_sf_{name}"
return (self.copy(name_in_target=name_in_target,
arg_id_to_dtype=constantdict(arg_num_to_dtype)),
clbl_inf_ctx)
Comment on lines +861 to +867
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

with_types does not validate the expected signature for GSL special functions (e.g. hyperg_2F1 is a fixed-arity function) and currently sets the return dtype to the dtype of the last argument via max(arg_num_to_dtype). This can mis-specialize the callable (wrong return type, accepting wrong number/types of args) and can also raise a ValueError on max() for invalid/empty calls. Consider explicitly enforcing the correct arity and specializing both argument and return dtypes to the actual GSL signature (typically double), relying on Loopy's casting to convert inputs when needed.

Copilot uses AI. Check for mistakes.
Comment on lines +861 to +867
Copy link
Owner

Choose a reason for hiding this comment

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

There's probably some argument type validation missing here. I'm willing to believe that this uses overloads for types (C11-style), but that's probably still a finite set?


def generate_preambles(self, target):

Check warning on line 869 in loopy/target/c/__init__.py

View workflow job for this annotation

GitHub Actions / basedpyright

Type annotation is missing for parameter "target" (reportMissingParameterType)

Check warning on line 869 in loopy/target/c/__init__.py

View workflow job for this annotation

GitHub Actions / basedpyright

Method "generate_preambles" is not marked as override but is overriding a method in class "ScalarCallable" (reportImplicitOverride)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add the missing annotations (here and below)? (bpr will also complain I think.)

# Base GSL math header
yield ("40_c_gsl_math", "#include <gsl/gsl_math.h>")

# Function-specific headers for GSL special functions
if self.name == "hyperg_2F1":
# gsl_sf_hyperg_2F1 is declared in <gsl/gsl_sf_hyperg.h>
yield ("41_c_gsl_sf_hyperg", "#include <gsl/gsl_sf_hyperg.h>")


def get_c_callables():
"""
Returns an instance of :class:`InKernelCallable` if the function
Expand All @@ -866,6 +896,14 @@
func_ids = ["bessel_jn", "bessel_yn"]
return {id_: GNULibcCallable(id_) for id_ in func_ids}


def get_gsl_callables():
Copy link
Owner

Choose a reason for hiding this comment

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

Add this to the documentation somewhere.

# Support special functions from
# https://www.gnu.org/software/gsl/doc/html/specfunc.html
func_ids = ["hyperg_2F1"]
return {id_: GSLCallable(id_) for id_ in func_ids}
Comment on lines +900 to +904
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

New callable support (hyperg_2F1 via GSL) is introduced here, but there is no corresponding test (compare the existing test_glibc_bessel_functions coverage). Adding a test that compiles/runs an ExecutableCWithGNULibcTarget kernel calling hyperg_2F1 and validates against a reference (e.g. scipy.special.hyp2f1/mpmath), with a skip if GSL isn't available, would help prevent regressions.

Copilot uses AI. Check for mistakes.


# }}}


Expand Down Expand Up @@ -1612,6 +1650,7 @@
def known_callables(self):
callables = super().known_callables
callables.update(get_gnu_libc_callables())
callables.update(get_gsl_callables())

Check warning on line 1653 in loopy/target/c/__init__.py

View workflow job for this annotation

GitHub Actions / basedpyright

Type of "update" is partially unknown   Type of "update" is "Overload[(m: SupportsKeysAndGetItem[Unknown, Unknown], /) -> None, (m: SupportsKeysAndGetItem[str, Unknown], /, **kwargs: Unknown) -> None, (m: Iterable[tuple[Unknown, Unknown]], /) -> None, (m: Iterable[tuple[str, Unknown]], /, **kwargs: Unknown) -> None, (**kwargs: Unknown) -> None]" (reportUnknownMemberType)
Comment on lines 1652 to +1653
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

Registering GSL callables for CWithGNULibcASTBuilder makes ExecutableCWithGNULibcTarget kernels that use gsl_sf_* likely fail at build/link time unless the CCompiler is configured with the correct include_dirs and libraries (e.g. gsl, often also gslcblas). Consider providing a dedicated target/compiler preset for GSL (or updating this target) that wires in the required toolchain options, or otherwise surfacing a clear error when GSL headers/libs aren't available.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

This is a fair point. Here I'm abusing of the GNULibc classes, for which we should only register functions that depend on GNULibc. Should I create separate GSL counterparts?

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure I love either solution. I'm not sure I want a thicket of derived classes, but I agree that adding this by default here isn't awesome.

It might be best to leave it to the user to have their own derived class. Of course, this would be easier if known_callables lived in TargetBase, not the ASTBuilder. Let me see if that's a refactor that AI can do: #988.

return callables


Expand Down
Loading