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

Make tensor*tensor implementation easier to read. #16839

Merged
merged 1 commit into from Apr 4, 2024

Conversation

bangerth
Copy link
Member

@bangerth bangerth commented Apr 2, 2024

In particular, treat the two most common cases separately and perhaps gain a bit of efficiency that way if that helps compilers.

@bangerth
Copy link
Member Author

bangerth commented Apr 2, 2024

Part of #16465 .

reordered = TensorAccessors::reordered_index_view<0, rank_2>(src2);
TensorAccessors::contract<1, rank_1, rank_2, dim>(result,
src1,
reordered);
Copy link
Member

Choose a reason for hiding this comment

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

It took me considerable effort to come up with one generic implementation instead of special casing contractions of different template parameters fortran style.

And I fear right now that you are undoing a lot of this work.

Despite the rather intimidating template magic - the entire tensor contraction logic boils down to one loop where we do this:

     for (unsigned int i = 0; i < dim; ++i)
        result[i] += src1[i] * src2;

So, why not adding the vectorization bits there?

Relatedly, we could also ditch the Reordered IndexView for the special case of a "last and first index" contraction and provide a specialized, vectorized recursive template for that one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course that's the plan. I needed to create a place where I can put the vectorization in the next patch. This patch creates this place. It also, perhaps, makes the compiler's life easier for the two simplest cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Separately, though: I'm not ditching the generic implementation. There remain the various contract() functions that I don't intend to touch. They're the main users of your general approach, which I continue to admire. I just think that in these two specific cases, things are so simple that we can perhaps allow us to just write the obvious code :-)

@tamiko tamiko merged commit 3910325 into dealii:master Apr 4, 2024
16 checks passed
@bangerth bangerth deleted the tensor-product branch April 4, 2024 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants