-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
Transition to basix library for finite element definitions #296
Conversation
return [L.Comment(msg), L.Return(-1)] | ||
|
||
return generate_evaluate_reference_basis(L, data, parameters) | ||
def apply_dof_transformation(L, ir, parameters, reverse=False): |
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.
What exactly does this do? Is it only about permutations?
/// @param[in] cell_permutation An integer encoding the orientation of the | ||
/// cell's entities | ||
/// @param[in] dim The number of data items for each DOD | ||
int (*apply_dof_transformation)(double* data, uint32_t cell_permutation, |
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.
Related to my previous comment. If this is ever only about permutation, then why name suggest something more general?
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.
For more complicated situations such as face tangents, the DOF transformation is not a permutation, as it combines multiple DOFs to get the new DOF. It's probably more sensible though to call the functions permutations everywhere though to avoid confusion, so I'll rename stuff
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.
Actually, just noticed that there's apply_dof_transformation
and get_dof_permutation
and I guess it's useful to have different names to distinguish these
test/test_jit_elements.py
Outdated
@@ -7,13 +7,15 @@ | |||
import numpy as np | |||
import pytest | |||
|
|||
import ffcx | |||
import ffcx.codegeneration.jit | |||
# import ffcx |
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.
What is the point of this file now?
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.
Good question. I've removed it
This reverts commit ea7cbf7.
* add unpermute * un * revert * remove element factory name
return (points, weights) | ||
return (None, None) | ||
|
||
logging.exception(f"Unknown integral type: {integral_type}") |
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.
Is this ever hit?
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.
no. I've changed it to a RuntimeError as in the function below to give a stronger warning that something is wrong if this is hit
{{ | ||
{evaluate_reference_basis_derivatives} | ||
}} | ||
|
||
int transform_reference_basis_derivatives_{factory_name}( |
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.
Why is this method still in ufc?
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.
It's still used in dolfinx, but could possibly be more neatly replaced by implementing Piola mapping functionality in Basix
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've made an issue: #299
Refactor everything to use Basix in the place of FIAT. This PR is twinned with FEniCS/dolfinx#1324.
Close #277. Close #175. Close #176. Close #33.