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

Bugfix for periodic boundary conditions #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adediego
Copy link
Contributor

Currently, the following code throws an exception:

using CoherentStructures
using Distances

LL, UR = (0., 0.), (1., 1.)
gs = 10
ctx, _ = regularTriangularGrid((gs, gs), LL, UR);
predicate = (p1, p2) -> peuclidean(p1, p2, [1.0,Inf]) < 2e-10
bdata = BoundaryData(ctx, predicate, [])

adaptiveTOCollocationStiffnessMatrix(ctx, identity; on_cylinder=true, bdata)

It looks like this happens because in adaptiveTOCollocationStiffnessMatrix there is an attempt to avoid unnecessary evaluations of the flow by only applying it to the first num_real_points points. The actual redundant points however, may be distributed differently across the indices. Also, in adaptiveTOFutureGrid, it is assumed that flow_map_images is populated with the images of every point, so this is where the error is thrown.

This seems like the most direct fix. As the redundant points are typically on the boundary and thus very few, the effect on performance should be negligible.

It looks like the old version tried to avoid unnecessary
evaluations of the flow by only applying the flow to the
first `num_real_points` points. The actual redundant points
however, may be distributed differently across the indices.
Also, in adaptiveTOFutureGrid it is assumed that `flow_map` is
populated with the images of every point.
@adediego adediego requested a review from dkarrasch November 21, 2023 14:19
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.

1 participant