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

more strict file names / allow numbers in mesh file prefix #220

Merged
merged 3 commits into from
Dec 4, 2024

Conversation

dabele
Copy link
Contributor

@dabele dabele commented Dec 3, 2024

Main changes of this PR

Replay mode currently doesn't allow numbers in the mesh file prefix since the code is looking for the string "1" (if startdt = 1) anywhere in the file name, including the path. If the data files are directory1/mesh.init.vtk and directory1/mesh.dt1.vtk, the search picks directory1/mesh.init.vtk as the mesh for the first time step and complains about missing 0 or init file if initialization is required. (Error message: Starting from dt =1 but previous timestep ".init" or 0 was not found. Please make sure the relevant Mesh exists.) I think this only actually a problem if data initialization is enabled, otherwise the sorting of the list of mesh file names hides the bug.

With this change, the code is looking for ".dt1" instead, and only in the actual file name, not in the path.

Pro: Allow numbers in the mesh prefix path, e.g. resolution of the mesh, versions, dates, etc.

Con: Someone might rely on a less strict format, i.e. using mesh1.vtk as file names. The documentation doesn't explicitly forbid this, but it does talk about using the meshes generated by precice export. I guess the check could be relaxed a bit by checking for 1 instead of dt1, but not including the path.

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 think the documentation already covers this
  • I added a changelog entry in ./changelog-entries/ (create if necessary).
  • I updated potential breaking changes in the tutorial precice/tutorials/aste-turbine. => tutorials use meshes generated by precice export, so it's fine

Can add a test and changelog on request, not sure this small change requires it.

replay mode looks for ".dtN" instead of just "N" in the mesh file name
@davidscn
Copy link
Member

davidscn commented Dec 4, 2024

I guess the check could be relaxed a bit by checking for 1 instead of dt1, but not including the path.

An alternative would be to check for the digit in front of the suffix (vtk/vtp or similar). When exporting every iteration (every-iteration) in preCICE, there is no dt anymore in the naming scheme, but this might change as well. Implicit coupling and replay mode is, however, anyway a bit of a grey area and not really tested. I would say: let's go with the strict variant and relax it later on if there are complains.

Would you mind adding a changelog entry?

Copy link
Member

@davidscn davidscn left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution :)

@davidscn davidscn merged commit 08caa9e into precice:develop Dec 4, 2024
3 checks passed
@dabele dabele deleted the find-filemesh-file-more-robust branch December 4, 2024 11:04
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