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.

Sign up for GitHub

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

Jump to bottom

Fix voting algorithm #3293

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Fix voting algorithm #3293

wants to merge 20 commits into from
+230 −34

Conversation

ReubenHill
Copy link
Contributor

@ReubenHill ReubenHill commented Dec 14, 2023 •
edited
Loading

Fixes #3151

@ReubenHill
Voting algorithm: prioritise owned cells
433b004 
Not yet implemented second cell search
@ReubenHill ReubenHill marked this pull request as draft December 14, 2023 10:57
@ReubenHill ReubenHill force-pushed the ReubenHill/voting-algorithm-fix branch from d33cca9 to aa5aaca Compare December 15, 2023 09:39
@ReubenHill ReubenHill mentioned this pull request Dec 15, 2023
Open
@ReubenHill
Copy link
Contributor Author

ReubenHill commented Dec 30, 2023 •
edited
Loading

Todo:

  • Work out why single parallel immersedsphere test is failing. It's a tolerance test so it could be something to do with how cell location works on immersed manifolds (therefore requiring a tolerance reduction). Strangely, it passes locally on my mac. Could this be because the second set of locate_cells_ref_coords_and_dists calls are not done collectively?
  • Check speed relative to old voting algorithm

@ReubenHill
Copy link
Contributor Author

For reference, the voting algorithm as implemented here is described in section 7.5 of my thesis
Nixon-Hill-RW-2024-PhD-Thesis.pdf.pdf

@connorjward connorjward mentioned this pull request Oct 18, 2024
Closed
@connorjward connorjward self-assigned this Oct 23, 2024
@connorjward
Copy link
Contributor

I will take this on and try and get it merged. I have identified a couple of issues with this, and have some ideas for more scalable approaches, but I think this will fix the common failure case that people are seeing at the moment.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 23, 2024 •
edited
Loading

TestsPassed ✅Skipped ⏭️Failed ❌
Firedrake complex8082 ran6500 passed1582 skipped0 failed

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 23, 2024 •
edited
Loading

TestsPassed ✅Skipped ⏭️Failed ❌
Firedrake real8088 ran7302 passed786 skipped0 failed

@connorjward connorjward mentioned this pull request Oct 23, 2024
Open
@connorjward connorjward marked this pull request as ready for review October 23, 2024 16:24
@ReubenHill
Copy link
Contributor Author

ReubenHill commented Oct 23, 2024 via email

@connorjward
Copy link
Contributor

Thanks for taking this on. I hope the notes I made before leaving are useful to you!

Yes thank you. I have also really appreciated your detailed comments in this part of the code!

It looks like tests are passing following a merge of master (and a tiny conflict) so hopefully this will get merged next Weds.

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

@connorjward connorjward

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: Cross mesh interpolation breaks for parallel nested meshes
2 participants
@ReubenHill @connorjward