dialects: (tensor) add init to tensor.from_elements#5373
dialects: (tensor) add init to tensor.from_elements#5373positr0nium wants to merge 16 commits intoxdslproject:mainfrom
tensor.from_elements#5373Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5373 +/- ##
=======================================
Coverage 86.30% 86.31%
=======================================
Files 404 404
Lines 57555 57560 +5
Branches 6684 6685 +1
=======================================
+ Hits 49675 49682 +7
+ Misses 6322 6321 -1
+ Partials 1558 1557 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
…r_operations_clean
…ium/xdsl into tensor_operations_clean
superlopuh
changed the title
tensor.from_elements
superlopuh
changed the title
tensor.from_elementstensor.from_elements
xdsl/dialects/tensor.py
Outdated
| traits = traits_def(NoMemoryEffect()) | ||
|
|
||
| def __init__(self, *elements: SSAValue, result_type: Attribute | None = None): | ||
| if len(elements) == 0: |
There was a problem hiding this comment.
| if len(elements) == 0: | |
| if not elements: |
There was a problem hiding this comment.
But also I'm not sure that it's possible to call this function with an empty list? Can you please either add a test that triggers the ValueError or remove this check?
There was a problem hiding this comment.
Ah no I'm wrong, you've already added the test!
There was a problem hiding this comment.
I would still recommend using truthiness for checking whether a Sequence is empty or not, as it's generally an O(1) operation, as opposed to len, which is potentially an O(n) operation.
There was a problem hiding this comment.
I just remembered the trick to guarantee that the number of elements is not 0:
def foo(head: int, *tail: int):
els = (head, *tail)Then calling this without the first element is a type error, and we don't need to check it dynamically.
There was a problem hiding this comment.
Not sure but I think the testing pipeline doesn't like the hack. It passes locally for me though.
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
|
Hi @positr0nium, do you have time to iterate in this in the next few weeks? |
…FromElementsOp.__init__
|
Hi @superlopuh a bit of time is now available :) Let's finish this soon! |
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
…g of FromElementsOp
|
My local pre-commit applies formatting changes that the remote pre-commit doesn't seem to appreciate. Any ideas on how to fix this? |
|
I would recommend running make pyright, then trying again. |
|
If that doesn't work let's debug together on Zulip. |
| if result_type is None: | ||
| elem_type = element_0.type | ||
| result_type = TensorType(elem_type, (len(elements) + 1,)) | ||
|
|
||
| super().__init__(operands=[(element_0,) + elements], result_types=[result_type]) |
There was a problem hiding this comment.
| if result_type is None: | |
| elem_type = element_0.type | |
| result_type = TensorType(elem_type, (len(elements) + 1,)) | |
| super().__init__(operands=[(element_0,) + elements], result_types=[result_type]) | |
| elements = (element_o,) + elements | |
| if result_type is None: | |
| result_type = TensorType(element_0.type, (len(elements),)) | |
| super().__init__(operands=(elements,), result_types=[result_type]) |
There was a problem hiding this comment.
I messed up in the UI resolving this, can you please revert these changes locally?
| def test_from_elements_type_consistency(): | ||
| """Test that FromElementsOp enforces type consistency among elements.""" | ||
| a_i64 = create_ssa_value(i64) | ||
| b_f64 = create_ssa_value(f64) | ||
|
|
||
| from xdsl.utils.exceptions import VerifyException | ||
|
|
||
| # This should raise an assertion error due to type mismatch | ||
| res = FromElementsOp(a_i64, b_f64) | ||
| with pytest.raises(VerifyException): | ||
| res.verify() | ||
|
|
||
|
|
||
| def test_from_elements_empty_list(): | ||
| """Test FromElementsOp with an empty list.""" | ||
| # Empty lists should raise a TypeError since we can't infer element type | ||
|
|
||
| with pytest.raises(TypeError): | ||
| FromElementsOp() | ||
|
|
There was a problem hiding this comment.
let's remove these tests, we should test for this in the dialect lit tests
| def test_from_elements_single_element(): | ||
| """Test FromElementsOp with a single element in a list.""" | ||
| a = create_ssa_value(f64) | ||
|
|
||
| res = FromElementsOp(a) | ||
|
|
||
| # Check that the result is a 1-D tensor with 1 element | ||
| assert isinstance(res.result.type, TensorType) | ||
| assert res.result.type.get_shape() == (1,) | ||
| assert res.result.type.element_type == f64 | ||
| assert len(res.elements) == 1 | ||
| assert res.elements[0] is a | ||
|
|
||
|
|
||
| def test_from_elements_different_numeric_types(): | ||
| """Test FromElementsOp with different numeric element types.""" | ||
| # Test with f64 | ||
| a_f64 = create_ssa_value(f64) | ||
| b_f64 = create_ssa_value(f64) | ||
|
|
||
| res_f64 = FromElementsOp(a_f64, b_f64) | ||
| assert res_f64.result.type.element_type == f64 | ||
| assert res_f64.result.type.get_shape() == (2,) | ||
|
|
||
| # Test with i64 | ||
| a_i64 = create_ssa_value(i64) | ||
| b_i64 = create_ssa_value(i64) | ||
|
|
||
| res_i64 = FromElementsOp(a_i64, b_i64) | ||
| assert res_i64.result.type.element_type == i64 | ||
| assert res_i64.result.type.get_shape() == (2,) |
There was a problem hiding this comment.
these seem like duplicate tests from above?
2 participants
Implemented the constructor of
tensor.FromElementsOpand set the NoMemoryEffect trait as detailed here.