Skip to content
This repository was archived by the owner on Nov 11, 2021. It is now read-only.

Add fvm advection stencil to documentation#79

Draft
tehrengruber wants to merge 5 commits intomasterfrom
doc_fvm_advect
Draft

Add fvm advection stencil to documentation#79
tehrengruber wants to merge 5 commits intomasterfrom
doc_fvm_advect

Conversation

@tehrengruber
Copy link
Contributor

This is just a draft for discussion. The gtscript syntax used here is not supported right now.

@tehrengruber tehrengruber self-assigned this Oct 14, 2020
@GridTools GridTools deleted a comment from github-actions bot Oct 14, 2020
@github-actions
Copy link

Documentation preview

@tehrengruber tehrengruber marked this pull request as draft October 21, 2020 11:01
@tehrengruber tehrengruber requested a review from havogt October 21, 2020 11:01
Copy link
Contributor

@havogt havogt left a comment

Choose a reason for hiding this comment

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

I think we should remove all nodes we currently don't use (YAGNI).
This also remove a design flaw because at nir level we don't want to set how many neighbors we have.

init: Optional[Expr] # TODO: use in gtir to nir lowering for reduction var


class TensorLocalVar(LocalVar):
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just do what we need now and remove this extra step for now.

init: Optional[List[Expr]]


class LocalFieldVar(TensorLocalVar):
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should pass max_size for constructing this node. If the TensorLocalVar is gone, we have no problem and no smart patterns.

#class IndexAccess(Access): # TODO(tehrengruber): use for TensorLocalVar
# indices: List[int]

class LocationLocalIdAccess(Access):
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this for now.

)

def visit_NeighborLoopLocationAccess(self, node: nir.NeighborLoopLocationAccess, **kwargs):
return usid.IndexAccess(name=node.name+"_neigh", location_type=node.location_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

NeighborLoop is declaring a symbol which we want to use here. I think the correct way is node.name -> nir.NeighborLoop -> usid.NeighborLoop -> iter_var. It's probably something we should discuss with @egparedes how that could work with the symbol table.


def visit_NeighborLoop(self, node: nir.NeighborLoop, **kwargs):
return usid.NeighborLoop(
iter_var=node.name+"_neigh",
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to find the right pattern, see marker below.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants