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

Add secondary data to exsting QN tests #2006

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

Conversation

Fujikawas
Copy link
Contributor

Main changes of this PR

One data set is added to the integrated QN tests and it is supposed to be accelerated as secondary data.

Among the Quasi-Newton integration tests, those with ILS acceleration could pass and those with IMVJ acceleration would fail since there is no proper acceleration to the secondary data.

Motivation and additional information

To increase test coverage of secondary data in QN methods as mentioned in issue #1996.

Author's checklist

  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • I squashed / am about to squash all commits that should be seen as one.

Reviewers' checklist

  • Does the changelog entry make sense? Is it formatted correctly?
  • Do you understand the code changes?

Copy link
Member

@uekerman uekerman 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 in general. Thanks for the push 👍

We no longer have any tests that have no secondary data now. But we could add use all data as primary. You could copy from how things are done here: https://github.com/precice/precice/blob/develop/tests/serial/mapping-nearest-projection/MappingNearestProjectionEdges.cpp

tests/quasi-newton/parallel/TestQN2.xml Outdated Show resolved Hide resolved
tests/quasi-newton/helpers.cpp Show resolved Hide resolved
tests/quasi-newton/helpers.cpp Show resolved Hide resolved
tests/quasi-newton/helpers.cpp Show resolved Hide resolved
@Fujikawas
Copy link
Contributor Author

We no longer have any tests that have no secondary data now.

The parallel/TestQNXemptyPartition tests are still (mostly) with only primary data.

@uekerman
Copy link
Member

The parallel/TestQNXemptyPartition tests are still (mostly) with only primary data.

True 👍 but we could still easily increase test coverage with the boolean switch explained above.

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.

None yet

2 participants