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

docs: Fixed typos in ADRs and a docstring #1627

Merged
merged 1 commit into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/development/ADRs/0001-Field_View_Frontend_Design.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ Parses from Python AST into PAST.
Lowers from PAST to Iterator IR.

**Field operator out argument slicing**
The lowering contains some complex logic to emulate slicing of fields in order to restrict the output domain of a field operator call. This contradics guiding principle (3) and should be removed in the future. The concept of specifying the domain for Field operators was not investigated during the frontend design as we concentrated on the beautified iterator dialect where the output domain is explicitly given when lifting a local operator to a field operator. The alternative to also explicitly specify the domain for calls to field operators was rejected as it:
The lowering contains some complex logic to emulate slicing of fields in order to restrict the output domain of a field operator call. This contradicts guiding principle (3) and should be removed in the future. The concept of specifying the domain for Field operators was not investigated during the frontend design as we concentrated on the beautified iterator dialect where the output domain is explicitly given when lifting a local operator to a field operator. The alternative to also explicitly specify the domain for calls to field operators was rejected as it:

- would require introducing significantly more syntax in order to specify domains, which was not only infeasible to implement in time, but there also didn't exist an accepted syntax to do so.
- no intuitive syntax was found to do this
Expand Down
2 changes: 1 addition & 1 deletion docs/development/ADRs/0002-Field_View_Lowering.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,4 @@ Since temporary variables are no longer inlined, the renaming that happens in th

## Operator signature FOAST <-> ITIR

On iterator level the arguments to all stencils / functions used inside a fencil closure need to be iterators (due to the compiled backend using a single SID composite to pass the arguments). Following the FOAST value <-> ITIR value, FOAST field <-> ITIR iterator correspondence, all field operator arguments whose type on FOAST level is a value, i.e. scalar or composite thereof, are expected to be values on ITIR level by the rest of the lowering. As a consequence we transform all values into iterators before calling field operators (to satisfy the former constraint) and deref them immediately inside every field operator (to satify the latter constraint).
On iterator level the arguments to all stencils / functions used inside a fencil closure need to be iterators (due to the compiled backend using a single SID composite to pass the arguments). Following the FOAST value <-> ITIR value, FOAST field <-> ITIR iterator correspondence, all field operator arguments whose type on FOAST level is a value, i.e. scalar or composite thereof, are expected to be values on ITIR level by the rest of the lowering. As a consequence we transform all values into iterators before calling field operators (to satisfy the former constraint) and deref them immediately inside every field operator (to satisfy the latter constraint).
2 changes: 1 addition & 1 deletion docs/development/ADRs/0009-Compiled-Backend-Integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ The interface for step two is defined in the `fencil_processors.pipeline.Binding

The output of this step is a `BindingsModule` instance and also safely hashable.

This would could be usefeul as an endpoint to use the Python bindings as an example for handcrafted bindings to other languages or for distribution of the generated bindings in a self-contained library with it's own build system.
This would could be useful as an endpoint to use the Python bindings as an example for handcrafted bindings to other languages or for distribution of the generated bindings in a self-contained library with it's own build system.

**Step 3a:**

Expand Down
2 changes: 1 addition & 1 deletion docs/development/ADRs/0010-Domain_in_Field_View.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ the domain was deduced relative to _the_ `out` argument. We decided to deduce th

## Context

The compute domain in field view is deduced relative to the shape of fields in the `out` argument. This is straigh-forward for a single field.
The compute domain in field view is deduced relative to the shape of fields in the `out` argument. This is straight-forward for a single field.
For multiple fields we would need to check at call-time if all fields are sliced in the correct way.
Additionally, expressing absolute domains is desirable for some use-cases.
In summary, we need to revisit the domain handling in field view.
Expand Down
4 changes: 2 additions & 2 deletions docs/development/ADRs/0011-On_The_Fly_Compilation.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ _edit 2023-03-29_

`NamedStepSequence` solves the following problem:

By making it possible to quickly create a variant of an existing workflow (using `workflow.replace`), they make it unneccessary to escalate sub-workflow configuration.
By making it possible to quickly create a variant of an existing workflow (using `workflow.replace`), they make it unnecessary to escalate sub-workflow configuration.

Coming back to the above example of `ExampleWorkflow`, instead of:

Expand Down Expand Up @@ -277,7 +277,7 @@ This greatly simplifies accessing already compiled and cached (across runs) GT4P

Whereas when using the `BuildData` one only requires to run steps 1) and 2) to get the cached project folder and the rest can be accessed from there. These steps can be skipped as well by implementing more aggressive caching (for production environments where the GT4Py source is kept constant).

Looking at the build systems implemented at the time of writing, `CMakeProject` and `CompiledbProject`, one might wrongly conclude that existence of certain directories or files suffice. This is only the case because `CompiledbProject` is based on `CMakeProject` and they therefore share some implementation details, which are by no means guaranteed to be standardizeable across multiple conceptually different build systems.
Looking at the build systems implemented at the time of writing, `CMakeProject` and `CompiledbProject`, one might wrongly conclude that existence of certain directories or files suffice. This is only the case because `CompiledbProject` is based on `CMakeProject` and they therefore share some implementation details, which are by no means guaranteed to be standardizable across multiple conceptually different build systems.

#### Revise If

Expand Down
2 changes: 1 addition & 1 deletion docs/development/ADRs/0014-DaCe_backend.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ After a cycle, we have a working implementation of the iterator IR to DaCe SDFG

**Solution**: The iterator type inference pass should provide the concrete type of all value-expressions.

#### Partial suppot for iterator IR grammar
#### Partial support for iterator IR grammar

- First order functions: in DaCe SDFG's, first order functions could be represented by associating a nested SDFG (function equivalent in DaCe) with an access node (variable equivalent in DaCe), which is not a thing as far as I know. There is no practical solution to support this in the SDFG lowering, such ITIR will always be rejected and should not be generated from ITIR passes.
- Lambdas: in DaCe SDFG's, an immediate call to an instantiation of a lambda function can be represented as a nested SDFG. Currently, the DaCe backend does not support this, but the solution should be fairly straightforward. Lambda support is necessary as they are essential for CSE in ITIR.
Expand Down
2 changes: 1 addition & 1 deletion docs/development/ADRs/0017-Toolchain-Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ At the time of creation of this document, at least one additional toolchain is a

- Some ways of configuring toolchains involve configuring multiple components in synch
- Some ways of configuring toolchains involve switching out or nesting toolchain steps
- What configuration options are avaiable will depend on what toolchain will be used and how that is configured.
- What configuration options are available will depend on what toolchain will be used and how that is configured.
- Hierarchical configuration defaults and overrides can be confusing from a user perspective.
- Leaving the configuration interface completely up to toolchain developers could lead to a confusing ad fragmented user experience.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def visit_Stencil(self, node: oir.Stencil, **kwargs: Any) -> oir.Stencil:


class WriteBeforeReadTemporariesToScalars(TemporariesToScalarsBase):
"""Replaces temporay fields that are always written before read by scalars.
"""Replaces temporary fields that are always written before read by scalars.

Note that temporaries used in horizontal regions in a single horizontal execution
may not be scalarized.
Expand Down
Loading