Skip to content

Comments

Fall back gracefully when sum factorisation fails.#207

Draft
dham wants to merge 1 commit intomainfrom
dham/sum_factorisation_fallback
Draft

Fall back gracefully when sum factorisation fails.#207
dham wants to merge 1 commit intomainfrom
dham/sum_factorisation_fallback

Conversation

@dham
Copy link
Member

@dham dham commented Jan 12, 2026

The sum factorisation implementation only works with up to 6 indices in a contraction. There are
circumstances in which an interpolation kernel can exceed this. Rather than failing in this case, we
should not sum factorise.

The sum factorisation implementation only works
with up to 6 indices in a contraction. There are
circumstances in which an interpolation kernel can
exceed this. Rather than failing in this case, we
should not sum factorise.
@connorjward
Copy link

As an alternative could we just have (here)

if len(sum_indices) > 6:
    logger.warning("That's a lot of indices you have there, this might take some time")

@dham
Copy link
Member Author

dham commented Jan 12, 2026

As an alternative could we just have (here)

if len(sum_indices) > 6:
    logger.warning("That's a lot of indices you have there, this might take some time")

Fair.

@dham
Copy link
Member Author

dham commented Jan 15, 2026

Result of Firedrake meeting discussion:

  • Make the maximum sum factorisation index be a variable.
  • If that variable is exceeded, don't sum factorise but do issue a warning. The warning should specify how to set the sum factorisation index.

@dham
Copy link
Member Author

dham commented Jan 16, 2026

Actually the proposed solution is not great. We'd have to pass the parameter through a whole load of code.

Additionally, this changes the generated code, so in principle we should change the signature of that code.

I think this is a mess for what is basically a hack in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants