-
Notifications
You must be signed in to change notification settings - Fork 55
feat[next]: Embedded domain construction from dimension comparison #2532
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
Changes from 1 commit
09dd736
5ec9bd3
d62e51a
9598eee
7f6393d
7205478
991d93a
99a9935
08eaec3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| --- | ||
| tags: [] | ||
| --- | ||
|
|
||
| # Limitations of embedded concat_where | ||
|
|
||
| - **Status**: valid | ||
| - **Authors**: Hannes Vogt (@havogt) | ||
| - **Created**: 2026-03-12 | ||
| - **Updated**: 2026-03-12 | ||
|
|
||
| In embedded execution, `concat_where` is, for now, limited to simple but common cases. | ||
|
|
||
| We do not support `concat_where` in cases | ||
|
|
||
| - where the domain would be infinite and therefore can't be represented as an ndarray, e.g. `concat_where(I < 0, 0.0, somefield)` where the scalar 0.0 would be broadcasted to a field reaching to -infinity; | ||
| - with multi-dimensional domains, e.g. `concat_where(I > 0 | J > 0, a, b)`. These cases need to be represented by a nested `concat_where(I > 0, a, concat_where(J > 0, a, b))`; | ||
|
|
||
| Additionally, we only support the most basic cases of non-consecutive domains: a tuple of `Domain`s resulting from e.g. `I != 0` or the equivalent `I < 0 | I > 0`. Operations on tuples of `Domain`s are not supported. | ||
|
|
||
| ## Context | ||
|
|
||
| `concat_where` requires expressing conditions like `I != i`, which produces two disjoint 1D domains (everything before index `i` and everything after). We need a way to represent these non-consecutive domains. | ||
|
|
||
| A complete implementation would require designing how to handle fields on non-hypercubic domains. Currently, `Domain` is a Cartesian product of per-dimension `UnitRange`s, which inherently describes hypercubic (rectangular) regions. Supporting arbitrary non-consecutive domains in multiple dimensions would mean fields could live on non-rectangular regions, requiring fundamental changes to field storage, slicing, and iteration. | ||
|
|
||
| ## Decision | ||
|
|
||
| We use a simple tuple-of-`Domain` representation for non-consecutive domains, restricted to: | ||
|
|
||
| - **1D only**: `Domain.__or__` raises `NotImplementedError` for multidimensional domains. | ||
| - **At most 2 domains**: `Dimension.__ne__` produces exactly 2 disjoint domains. No attempt is made to support arbitrary numbers of disjoint regions. | ||
|
|
||
| This is sufficient for the most common `concat_where` use case (`I != i` splits a dimension into two parts) without requiring a general solution for non-hypercubic domains/fields. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - `concat_where` works for 1D domain conditions, which covers the primary use case of vertical level exclusion. | ||
| - Combining multiple exclusions (e.g. `(I != 2) & (I != 5)`) is not supported because it would require a custom tuple type to implement the intersection/union operations. | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| ### General `concat_where` with multi-dimensional domain conditions | ||
|
|
||
| Implementation for multi-dimensional domain conditions (e.g. `(I != 2) | (K != 5)`) and full support for domain operations in `concat_where` would require | ||
|
|
||
| 1. **A `DomainTuple` class** with full algebra: a `tuple` subclass carrying `__and__`, `__or__`, `__rand__`, `__ror__` operators so that expressions like `tuple & Domain`, `Domain & tuple`, and `tuple | tuple` all work. | ||
|
|
||
| 2. **Normalization of domain tuples**: We need to design `DomainTuple` invariants, e.g. | ||
|
|
||
| - Should all domains be promoted to the same rank (missing dimensions filled with infinite ranges)? | ||
| - Should we reduce overlapping domains to non-overlapping via box subtraction? | ||
|
|
||
| Before implementing a complex `DomainTuple`, we should conclude on (if we want) a concept of non-consecutive fields. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -108,6 +108,54 @@ def __add__(self, offset: int) -> Connectivity: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __sub__(self, offset: int) -> Connectivity: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return self + (-offset) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __gt__(self, value: core_defs.IntegralScalar) -> Domain: # type: ignore[misc] # returns Domain, not bool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Domain(dims=(self,), ranges=(UnitRange(value + 1, Infinity.POSITIVE),)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __ge__(self, value: core_defs.IntegralScalar) -> Domain: # type: ignore[misc] # returns Domain, not bool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Domain(dims=(self,), ranges=(UnitRange(value, Infinity.POSITIVE),)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __lt__(self, value: core_defs.IntegralScalar) -> Domain: # type: ignore[misc] # returns Domain, not bool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Domain(dims=(self,), ranges=(UnitRange(Infinity.NEGATIVE, value),)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __le__(self, value: core_defs.IntegralScalar) -> Domain: # type: ignore[misc] # returns Domain, not bool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Domain(dims=(self,), ranges=(UnitRange(Infinity.NEGATIVE, value + 1),)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @overload | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __eq__(self, value: Dimension) -> bool: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @overload | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __eq__(self, value: core_defs.IntegralScalar) -> Domain: ... # type: ignore[overload-overlap] # intentionally returns Domain, not bool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @overload | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __eq__(self, value: object) -> bool: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __eq__(self, value: object) -> bool | Domain: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
havogt marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @overload | |
| def __eq__(self, value: Dimension) -> bool: ... | |
| @overload | |
| def __eq__(self, value: core_defs.IntegralScalar) -> Domain: ... # type: ignore[overload-overlap] # intentionally returns Domain, not bool | |
| @overload | |
| def __eq__(self, value: object) -> bool: ... | |
| def __eq__(self, value: object) -> bool | Domain: | |
| @overload | |
| def __eq__(self, value: Dimension) -> bool: ... | |
| @overload | |
| def __eq__(self, value: core_defs.IntegralScalar) -> Domain: ... # type: ignore[overload-overlap] # intentionally returns Domain, not bool | |
| def __eq__(self, value: Dimension | core_defs.IntegralScalar) -> bool | Domain: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want/need to support Dimension(...) == some_other_object comparison?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we go with your proposal, I should remove the else branch, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But does it makes sense to support Dimension == Any? Shouldn't that just raise a TypeError? Something like:
| @overload | |
| def __eq__(self, value: Dimension) -> bool: ... | |
| @overload | |
| def __eq__(self, value: core_defs.IntegralScalar) -> Domain: ... # type: ignore[overload-overlap] # intentionally returns Domain, not bool | |
| @overload | |
| def __eq__(self, value: object) -> bool: ... | |
| def __eq__(self, value: object) -> bool | Domain: | |
| if isinstance(value, Dimension): | |
| return self.value == value.value | |
| if isinstance(value, core_defs.INTEGRAL_TYPES): | |
| int_value = cast(core_defs.IntegralScalar, value) | |
| return Domain(dims=(self,), ranges=(UnitRange(int_value, int_value + 1),)) | |
| raise TypeError(....) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have cases, but I can try...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this works, but IIUC, this is the implemented behavior. If this works, a similar change could be applied to
__ne__There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mypy doesn't like it:
Overloaded function signatures 1 and 3 overlap with incompatible return types