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

Optional output for precice-aste-run #225

Merged
merged 4 commits into from
Feb 6, 2025
Merged

Optional output for precice-aste-run #225

merged 4 commits into from
Feb 6, 2025

Conversation

fsimonis
Copy link
Member

Main changes of this PR

Based on #210

This PR makes the --output argument of precice-aste-run of solver B optional, which disables writing the mapped data.
This can be useful in situations where only the runtime is measured and the result isn't needed.

The mapping tester also adds general>writeMapped to the setup which controls this for the entire case.

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.

@fsimonis fsimonis requested a review from davidscn January 28, 2025 14:37
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.

Looks good. What happens when I disable the output mesh writing/accuracy metrics and use the plotting scripts? Will it explode? As it is looking for the l2error.

@fsimonis
Copy link
Member Author

fsimonis commented Feb 5, 2025

Currently, only memory and runtime plots should be generated. The rest would crash. Maybe it would be an idea to change the plotting scripts to only plot the columns they find.

I'll open a separate PR

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.

Can you document this change in the main ASTE docs?

@fsimonis
Copy link
Member Author

fsimonis commented Feb 6, 2025

Rebased and cleaned up the PR. I'll merge after the CI passes.

@fsimonis fsimonis self-assigned this Feb 6, 2025
@fsimonis fsimonis merged commit fa47854 into develop Feb 6, 2025
7 checks passed
@fsimonis fsimonis deleted the optional-output-2 branch February 6, 2025 09:45
@fsimonis fsimonis mentioned this pull request Feb 6, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants