-
-
Notifications
You must be signed in to change notification settings - Fork 87
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
Pass the correct number of entries to writeData
#305
Conversation
Thanks for the PR! While it goes into the right direction, I still get the following (different) output from
Similarly for
For the
|
Had to adjust the buffer reading as well, as reading and writing share the same buffer.
I get the same, but I don't see how the changes here touch anything in the logical order of reading and writing. Seems to be related to @BenjaminRodenberg's work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes make sense, but I cannot yet confirm that it solves the issue. We need to first check if there is an issue in the core library.
ASSERTION FAILED
Location: void precice::impl::DataContext::mapData()
File: /home/gc/repos/precice/precice/src/precice/impl/DataContext.cpp:95
Expression: context.fromData->stamples().size() > 0
Rank: 0
Arguments: none
Note also the clang-format check failing.
Interface.C
Outdated
std::size_t nReadData = vertexIDs_.size() * precice_.getDataDimensions(meshName_,couplingDataReader->dataName()); | ||
// We could add a sanity check here | ||
// nReadData == vertexIDs_.size() * (1 + (dim_ - 1) * static_cast<int>(couplingDataReader->hasVectorData())); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should already be a unit test for getDataDimensions
, right? So, I don't see the added value. I would rather remove it.
std::size_t nReadData = vertexIDs_.size() * precice_.getDataDimensions(meshName_,couplingDataReader->dataName()); | |
// We could add a sanity check here | |
// nReadData == vertexIDs_.size() * (1 + (dim_ - 1) * static_cast<int>(couplingDataReader->hasVectorData())); | |
std::size_t nReadData = vertexIDs_.size() * precice_.getDataDimensions(meshName_,couplingDataReader->dataName()); | |
The exception above is only triggered for the partitioned-pipe. You could try one of the other examples. I wouldn't wait here for changes in precice/precice. |
For the assertions that you run into, are there already issues in preCICE? They look definitely like edge cases we have not yet covered with the waveforms. We should try to reproduce them with integration tests. Having the config in the issues could already be enough to create these tests. |
This is the already encountered precice/precice#1776 precice/precice#1751
Opened an issue: precice/precice#1803
@davidscn Why not an issue in the core library? Do you suspect we are reading/writing something wrong? |
I was referring to the lacking convergence, not to any assertion we hit. Once we change the substeps default back to |
Opened an issue for the convergence issues with substeps: precice/tutorials#373 |
Be aware that quasi Newton schemes are currently not supported, if For the FEniCS adapter the partitioned heat equations works with underrelaxation and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidscn all previously failing tutorials seem to work now. Feel free to merge.
Closes #303
I added now a return type to
CouplingDataUser::write()
to tell writeData how many data entries are actually relevant. Adding this return type is also a nice consistency check for theCouplingDataUser::write()
function and not uncommon for such functions.TODO list:
docs/
changelog-entries/
(create directory if missing)