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

Configurable partitioning algorithm in mapping tester #214

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

fsimonis
Copy link
Member

@fsimonis fsimonis commented Nov 7, 2024

Main changes of this PR

  • Add configurable partitioning algorithm
  • Make tests repeatable with topology-based partitioning

Author's checklist

  • I used the pre-commit hook and used pre-commit run --all to apply all available hooks.
  • I added a test to cover the proposed changes in our test suite.
  • I updated the documentation in docs/README.md.
  • I added a changelog entry in ./changelog-entries/ (create if necessary).
  • I updated potential breaking changes in the tutorial precice/tutorials/aste-turbine.

@@ -111,6 +111,7 @@ def main(argv):
print('Warning: outdir "{}" already exisits.'.format(outdir))
meshdir = os.path.join(outdir, "meshes")
function = setup["general"]["function"]
algorithm = setup["general"].get("partitioning", "meshfree")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the default ´"meshfree"´ to be backwards-compatible. It is an open question if this is desirable though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's ok.
precice-aste-partition --help lists available options. I would suggest to put a note for the kmeans variant, stating that the result is non-deterministic and not reproducible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment in the code or a message in the script output?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say "user visible", i.e., in the script output.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was more thinking of a comment here

help="""Change the algorithm used for determining a partition.
A meshfree algorithm works on arbitrary meshes without needing topological information.
A topology-based algorithm needs topology information
and is therefore useless on point clouds.
A uniform algorithm will assume a uniform 2d mesh laid out somehow in 3d and partition accordingly.""",

In the end, the behavior is not specific to the mapping-tester, but to the partitioning, right?

Copy link
Member Author

@fsimonis fsimonis Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see what you mean.

I think we need both as the help text needs to be read.

A warning in the partition script will fill the terminal when testing many rank combinations in the mapping tester.

@fsimonis fsimonis requested a review from davidscn November 8, 2024 09:22
@fsimonis fsimonis self-assigned this Dec 10, 2024
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