Detect native NaN values, warn about them

[inducer]

x-ref: inducer/pytools#287

This might be a good place, though the mapper obviously would need to be renamed.

cc @matthiasdiener

[matthiasdiener]

Hmm, for the simple test in

loopy/test/test_target.py

Lines 390 to 399 in 698f295

	def test_nan_support(ctx_factory, target):
	from pymbolic.primitives import NaN, Variable
	from loopy.symbolic import parse
	ctx = ctx_factory()
	queue = cl.CommandQueue(ctx)
	knl = lp.make_kernel(
	"{:}",
	[lp.Assignment(parse("a"), np.nan),

, parse (and thus FunctionToPrimitiveMapper) does not seem to be called with a nan argument. Is there a better place to perform this replacement?

[inducer]

On second thought, replacing "native" NaNs with pymbolic NaNs in one central location is probably not happening---there are too many possible "ways in", as you point out.

My next idea was to warn about them at code gen time, but it turns out that this is already being done:

loopy/loopy/target/c/codegen/expression.py

Lines 441 to 450 in 698f295

	elif np.isnan(expr):
	from warnings import warn
	warn("Encountered 'bare' floating point NaN value. Since NaN != NaN,"
	" this leads to problems with cache retrieval."
	" Consider using `pymbolic.primitives.NaN` instead of `math.nan`."
	" The generated code will be equivalent with the added benefit"
	" of sound pickling/unpickling of kernel objects.", stacklevel=1)
	from pymbolic.primitives import NaN
	data_type = expr.dtype.type if isinstance(expr, np.generic) else None
	return self.map_nan(NaN(data_type), type_context)

The analogous check for Python is missing though:

loopy/loopy/target/python.py

Lines 67 to 71 in 698f295

	def map_constant(self, expr, enclosing_prec):
	if isinstance(expr, np.generic):
	return repr(expr).replace("np.", "_lpy_np.")
	else:
	return repr(expr)

So that still needs to be added.

Did you see the warning above?

[matthiasdiener]

So that still needs to be added.

I can go ahead and this a warning there. Should this also do a pymbolic.primitives.NaN replacement, like in the C codegen?

Did you see the warning above?

I did see the warning in some tests; it could probably be improved though:

• also mention np.nan in addition to math.nan
• "sound pickling/unpickling of kernel objects" seems bit misleading - isn't pickling in fine general, just the cache retrieval is an issue?

[inducer]

Should this also do a pymbolic.primitives.NaN replacement, like in the C codegen?

Maybe just warn? I'm not sure Python code gen has really been tested for NaNs. If you're willing to take on that test, then sure, it's probably easiest to do NaN codegen via replacement.

I agree with your warning improvements.