DO NOT MERGE feat[next-dace]: Use nanobind Interface of DaCe

[philip-paul-mueller]

It is not fast only to see if it works.

[edopao]

An initial round of review. I think that the main point to discuss is the offset_provider argument.

[edopao]

after optimization they might no longer be needed and are removed

This only applies to scalar arguments, because they are represented as SDFG symbols. Unused symbols are pruned.
There were some changes over time, back and forward, whether to use scalars or symbols. The last reason for using symbols was that the gt4py toolchain was (is?) producing tuples of scalars for field domain range [prefix]_range_0, [prefix]_range_1. These scalars are used in domain expressions, therefore in memlet subsets and map ranges.

[philip-paul-mueller]

I reformulated it.

[edopao]

This text is not very clear. Besides, the line below makes me wonder why the function is needed?

In short: using it is most certainly an error.

[philip-paul-mueller]

Agreed this function does not make sense.
I ported it because it was there, but I remember that I thought about the old one "this function does not make sense".
And I also think that it can be made to work and we should simply turn it into an error.

[edopao]

Suggested change

	else:
	if eval_mode:
	assert not (isinstance(metric_level, str) or isinstance(compute_time_argument, str))
	return (metric_level, compute_time_argument)
	else:
	assert isinstance(metric_level, str) and isinstance(compute_time_argument, str)
	return f"{metric_level}, {compute_time_argument}, "
	if eval_mode:
	assert not (isinstance(metric_level, str) or isinstance(compute_time_argument, str))
	return (metric_level, compute_time_argument)
	else:
	assert isinstance(metric_level, str) and isinstance(compute_time_argument, str)
	return f"{metric_level}, {compute_time_argument}, "

[edopao]

Maybe a bit more clear, not so important.

[philip-paul-mueller]

I kind of agree that it is now would be nicer.
However, it would not look nice if we implement GTFN.
So I would keep it the way it is.

[edopao]

Here I am very confused. Do you need gtx_common.OffsetProvider or gtx_common.OffsetProviderType? The first has ndarray, the second not. Tome it seems that you need gtx_common.OffsetProvider.

Also, I do not understand why you are passing all connectivities, and not only the connectivities which are used (non transient arrays). This is related to another comment I have above.

[philip-paul-mueller]

To answer your first question that I do not fully understand myself either.
However, there are two things here. In eval mode you do not need the OffsetProviders but not in generation mode, where you only need the structure of the offset providers.
However, I think that we should remove the eval mode entierly, it was funny to implement but it does not really serves a purpose.
The second part to your first question is, in build_sdfg_from_gtir() the type annotation is OffsetProviderType, however, for generate_sdfg() the type annotation is OffsetProvider.
But I will remove the eval mode, this will also remove some complexity.

Your second question.
I do not need to pass them, in the generation of the user_args the unused are filtered out, but must be passed (or at least something).
I think I will change that such that None is passed in that case.

[edopao]

Can we hard-code eval_mode=Trueand have a wrapper function that evaluates the entire generated string instead of evaluating piece by piece?

[edopao]

Also, am I wrong to say that eval_mode=True would allow to defer passing the offset_provider to call time, thus we could generate the bindings in the binding stage? You do not need to know the keys in the offset_provider table, all you need is the list of non-transient arrays used for connectivities. See in baseline:

def _parse_gt_connectivities(
    code: codegen.TextBlock, sdfg_arglist: dict[str, dace.data.Data]
) -> None:
    for sdfg_arg_index, (arg_name, arg_desc) in enumerate(sdfg_arglist.items()):
        if gtx_dace_args.is_connectivity_identifier(arg_name):
            assert isinstance(arg_desc, dace.data.Array)
            assert len(arg_desc.shape) == 2
            assert isinstance(arg_desc.shape[1], int) or str(arg_desc.shape[1]).isdigit()

[philip-paul-mueller]

No, the idea of eval_mode was that you could actually debug the code, because the Python debugger could not handle dynamically generates code.
However, this idea was stupid.

I am actually not sure if you can move it to call time.
Because you essentially need Program.params and the offset providers.

Furthermore, now the offset providers are also passed by their position.
Thus you must know their order and not just which exists and which not.
The user_args interface does not support **kwargs.

I hoped this helped.

[edopao]

I think you can avoid passing offset_provider_type, for dace, see below.
As for GTFN backend, it does not have offset_provider either in the binding stage.

[edopao]

Suggested change

	if backend == "dace" and sdfg is not None:
	sdfg_arglist = sdfg.arglist()
	needed_offset_providers = {
	table_name
	for table_name in offset_provider_type
	if gtx_sdfg_args.connectivity_identifier(table_name) in sdfg_arglist
	}
	if sdfg is not None:
	needed_offset_providers = {aname for aname, _ in sdfg_arglist.items() if gtx_dace_args.is_connectivity_identifier(aname)}

[philip-paul-mueller]

I agree this is nicer.

[edopao]

Suggested change

	def _process_offset_providers(
	offset_provider_type: gtx_common.OffsetProviderType,
	needed_offset_providers: Optional[Container[str]],
	) -> str:
	if needed_offset_providers is None:
	needed_offset_providers = set(offset_provider_type.keys())
	# Assumes that the order of the offset providers is stable.
	processed_offset_providers = ""
	for table_name in offset_provider_type:
	if table_name in needed_offset_providers:
	processed_offset_providers += f"(offset_provider['{table_name}'].ndarray, (0, 0))"
	else:
	processed_offset_providers += "None"
	processed_offset_providers += ", "
	return processed_offset_providers
	def _process_offset_providers(
	needed_offset_providers: List[str],
	) -> str:
	# Assumes that the order of the offset providers is stable.
	processed_offset_providers: list[str] = []
	for table_name in needed_offset_providers:
	processed_offset_providers.append(f"(offset_provider['{table_name}'].ndarray, (0, 0))")
	return ", ".join(processed_offset_providers=

[philip-paul-mueller]

This does not work.
The reason is because the order in needed_offset_providers is not stable and we have to pass the offset provders by their position.
So you must iterate over something that has the same order as the offset provider that is finally used as call time.
Furthermore, GTFN passes always all offset providers.

I looked at GTFN and there they "ship" all offset providers through the entry_point property .
I agree there is a way, it is just not super nice.

[edopao]

Suggested change

	# The list of the SDFG arguments", i.e. the non transient arrays. Note that
	# tuple arguments are flatten and their name is mangled and no longer matches
	# the name in the signature of the field operator / program. Also note that
	# scalar arguments after optimization might no longer be needed and have been
	# pruned from the SDFG. Also the domain related symbols are not included.
	# NOTE: The dispatch code does not use it, instead the `user_args` are used.
	return [arg_name for arg_name, _ in sdfg_args]
	# The list of the SDFG arguments, i.e. the global arrays, scalars and free symbols.
	# Note that tuple arguments are flattened and their name is mangled and no longer
	# matches the name in the signature of the field operator / program.
	# Also note that unused SDFG symbols are pruned, so some scalar arguments
	# used in symbolic domain expressions might not appear in this signature.
	# NOTE: The dispatch code does not use it, instead the `user_args` are used.
	return [arg_name for arg_name, _ in sdfg_args]

[philip-paul-mueller]

Are you sure about the last sentence?
I would say that symbols might appear in this signature that are not actually needed (and because of that are removed from the C-API).