inducer / inducer/grudge

Operator interface

Open
#175 4 comments 0 reactions 0 assignees View on GitHub
design decisions documentation question
Dominant language
Python
Stars
14
Forks
18
Avg merge
26m
Merged PRs (30d)
1

Description

@alexfikl brought up a great point about the current operator interface: https://github.com/inducer/grudge/pull/172#discussion_r729138306

As already pointed out, grudge operators typically have a function signature like: `operator(dcoll, *args)`, such as: https://github.com/inducer/grudge/blob/2af3528391bf4cb07f5aacf43f7c6afcc18f693a/grudge/op.py#L437

The issue here is that we're not quite enforcing anything on `*args`. So, for example, if a user passes in `(vec, dd)` (instead of the expected `(dd, vec)` ordering), the code will break without actually catching this problem. Currently, we have the docstrings elaborate on `*args`, such as: https://github.com/inducer/grudge/blob/2af3528391bf4cb07f5aacf43f7c6afcc18f693a/grudge/op.py#L438-L441

We could strengthen the checks inside each function. However, I can see this becoming a bit unwieldy. We could also revamp the interface and impose a fixed signature for each function (meaning we _always_ require args: `dcoll, dd, vec` --- no more `*args`).

I thought I'd raise this issue so we can discuss this. I don't think this is a huge problem right now, but definitely worth noting.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.