Debug meshFields backend to fit 1.0.0 API - #346
Conversation
| #else | ||
| // Computation in barycentric_from_global sometimes produce values | ||
| // slightly outside [0, 1] for points on or near element boundaries. | ||
| for (int j = 0; j < dim_; ++j) { |
There was a problem hiding this comment.
@cwsmith @Joshua-Kloepfer This code is required due to the assert statements in MeshFields. I'm wondering if those asserts should have tolerances to allow for floating point computations, or if there is a better way to handle this.
There was a problem hiding this comment.
We can relax these checks:
to use Epsilon (1e-12) instead of MachinePrecision (1e-15) defined here:
Both seem somewhat arbitrary...
There was a problem hiding this comment.
A PR with a looser tolerance is here: SCOREC/meshFields#97
There was a problem hiding this comment.
I can remove the check here in pcms, but this seems to create a few incompatible versions between meshfields 1.0.0 and latest. Would that be a problem? If so, we'd probably need a patch on the meshfields side.
|
Afaik, we don't need to support old versions of meshfields.
…On August 3, 2026 4:40:03 PM EDT, Sichao25 ***@***.***> wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
@Sichao25 commented on this pull request.
________________________________
In src/pcms/field/evaluator/mesh_fields_backend.h<#346 (comment)>:
> for (int j = 0; j < (dim_ + 1); ++j)
coordinates_(index, j) = local[j];
+#else
+ // Computation in barycentric_from_global sometimes produce values
+ // slightly outside [0, 1] for points on or near element boundaries.
+ for (int j = 0; j < dim_; ++j) {
I can remove the check here in pcms, but this seems to create a few incompatible versions between meshfields 1.0.0 and latest. Would that be a problem? If so, we'd probably need a patch on the meshfields side.
—
Reply to this email directly, view it on GitHub<#346?email_source=notifications&email_token=AAH2DNUC7F4SSTWN7FRUKFL5ID2CHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBUHAZTMMBXHEZKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#discussion_r3707512803>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAH2DNVRW3BOBGBBNPZSVNT5ID2CHAVCNFSNUABFKJSXA33TNF2G64TZHMYTOMRXGU3DCMBSHNEXG43VMU5TKMBQGU2TSNBRHE2KC5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AAH2DNTB6TXE73MNZIF22LD5ID2CHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBUHAZTMMBXHEZKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android<https://github.com/notifications/mobile/android/AAH2DNXYYCLD774HYYAVV5T5ID2CHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBUHAZTMMBXHEZKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
With the tolerance update in meshFields, the assert clamping values into [0,1] has been removed. I've kept the |
|
@Sichao25 we may want to put the following into a separate PR, but my understanding is that MeshFields now include parametric coordinates of each DOFHolder. These should be used to construct global coordinates in the layout rather than the hard-coding we do for linear/quadratic currently. This should let us more seemlessly support alternative element types (e.g., reduced quintic) and higher degrees. |
|
Okay. I’ll put that in a separate PR. I think the error here is related to the MeshFields version in the pre-built dependency. @jacobmerson When you have a chance, would you mind checking whether we’re using a MeshFields version newer than 1.0.0? |
|
The prebuilt dependencies are on |
|
@hgangwar @Fuad-HH @abhiyan123 this PR will be a breaking change when we merge it. I will update my dependency build folder at the same time that merge this PR. |
3 participants
Change API to fit a few breaking changes introduced by MeshFields 1.0.0. The return value of
CreateLagrangeFieldis no longer the field itself, and the redundant barycentric coordinates have been removed.