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

Mixing data types and instantiated types in the user interface #1119

Closed
ali-ramadhan opened this issue Oct 30, 2020 · 1 comment · Fixed by #2991
Closed

Mixing data types and instantiated types in the user interface #1119

ali-ramadhan opened this issue Oct 30, 2020 · 1 comment · Fixed by #2991
Labels
abstractions 🎨 Whatever that means

Comments

@ali-ramadhan
Copy link
Member

More of a general issue that #1118 made me think about is that we've been mixing data types and instantiated types.

For example, we use data types when constructing grid topologies and specifying locations for abstract operations:

topology = (Periodic, Bounded, Bounded)
kinetic_energy @at (Cell, Cell, Cell) (u^2 + v^2) / 2

but instantiated types when specifying which architecture and advection scheme to use

model = IncompressibleModel(grid=grid, architecture=GPU(), advection=WENO5())

It seems there's some element of memorization to know which one to use when. Maybe this will confuse future users (certainly it confused us in #1118).

Changing this behavior would require a lot of refactoring but probably worth discussing whether changing makes sense or if we can make the user interface more robust and friendly to mismatches between data types and instantiated types?

@ali-ramadhan ali-ramadhan added the abstractions 🎨 Whatever that means label Oct 30, 2020
@glwagner
Copy link
Member

glwagner commented Nov 1, 2020

In some ways, omitting instantiation is a tempting hack that helps script-readability, but introduces slight complications in the source code and leads to confusion and memorization demands in the API...

I guess the confusion here is that there's no meaningful difference data type like Cell, Periodic, WENO5, GPU and their concrete realizations Cell(), Periodic(), etc. There's only a meaningful difference when concrete realizations can contain additional data.

We could simply require concrete realizations for specification everywhere. Then the semantics is uniform specifying types with additional data FPlane(f=1e-4) and types without data WENO5(). And we become ok with the syntax topology = (Periodic(), Periodic(), Bounded()).

We could alternatively auto-instantiate DataType where appropriate so that both CPU and CPU() work as expected. This wouldn't break the current API and may not be too much work, though could be harder to maintain moving forward.

Which is more friendly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abstractions 🎨 Whatever that means
Projects
None yet
2 participants