Repository navigation
compiler: Misc dtype lowering tweaks - #3090
Conversation
| values = p._arg_values(estimate_memory=estimate_memory, **kwargs) | ||
| # A replacement may supply defaults for other arguments, such as | ||
| # a ConditionalDimension's factor. Explicit overrides take precedence | ||
| args.update({k: v for k, v in values.items() |
There was a problem hiding this comment.
this is gonna get dropped
| for p in overrides: | ||
| args.update(p._arg_values(estimate_memory=estimate_memory, **kwargs)) | ||
| values = p._arg_values(estimate_memory=estimate_memory, **kwargs) | ||
| # A replacement may supply defaults for other arguments, such as |
There was a problem hiding this comment.
this looks like a bug in _arg_values
There was a problem hiding this comment.
it probably is, but as I wrote yesterday, I'm dropping this line for now. I moving the logic in copilot directly.
the issue here is when one supplies a factor= override while the supplied saved TimeFunction overrides still have the old factor
| if not dtypes or not np.issubdtype(base, np.complexfloating): | ||
| dtypes.update({base} - {None}) | ||
|
|
||
| dtypes = {dtypes_vector_mapper.get_base_dtype(i, i) for i in dtypes} |
There was a problem hiding this comment.
second place it's like that, why not move it inside infer_dtype?
| return max(dtypes, key=lambda i: np.dtype(i).itemsize, default=None) | ||
| dtype = max(dtypes, key=lambda i: np.dtype(i).itemsize, default=None) | ||
|
|
||
| return dtypes_vector_mapper[dtype, n] if n > 1 else dtype |
There was a problem hiding this comment.
likely need .get for safety?
| Extract the result dtype, retaining typed vector operands such as float2. | ||
| """ | ||
| # Arithmetic nodes may expose only their Functions' scalar precision | ||
| if not (expr.is_Add or expr.is_Mul or expr.is_Pow): |
There was a problem hiding this comment.
so what does it leave? THis is an off check
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3090 +/- ##
==========================================
+ Coverage 84.40% 84.45% +0.04%
==========================================
Files 260 260
Lines 58153 58262 +109
Branches 4948 4956 +8
==========================================
+ Hits 49086 49203 +117
+ Misses 8245 8229 -16
- Partials 822 830 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
eb64ab1 to
cb0f982
Compare
| if dtype is not None: | ||
| return dtype | ||
|
|
||
| dtypes = {extract_dtype(i) for i in expr.args} - {None} |
There was a problem hiding this comment.
This pattern pops up in a few places I think? Make a helper function if so?
There was a problem hiding this comment.
good catch added utility function
| @pytest.mark.parametrize('width', [1, 2]) | ||
| @pytest.mark.parametrize('reuse', [False, True]) | ||
| @pytest.mark.parametrize('initial', [0, 1]) | ||
| def test_index_derivative_vector(self, width, reuse, initial, monkeypatch): |
There was a problem hiding this comment.
Do we need to use monkeypatching in this file? Perhaps we want to update the coding style skills, since agents love using monkeypatch so far as I can tell...
Infer vector result types from operands, initialize each component, and reuse only type-compatible reduction accumulators. Preserve scalar precision queries for compiler metadata and FD weights.
Copilot now supplies saved Function replacements with matching ConditionalDimension factors, so the broad override filter is unnecessary.
fd8248b to
8741e35
Compare
No description provided.