Add gradient data write functionality#233
Conversation
MakisH
left a comment
MakisH
left a comment
There was a problem hiding this comment.
We kind of forgot about this PR in the middle of other changes. @davidscn we can also include this into the next release, we just need use/test cases.
Additional fields that could be useful would be pressure and velocity gradients in the FF module.
The changes make sense to me, and it looks like the API calls already correspond to preCICE v3.
Still needs to be documented and I would say we also need some kind of test case, to be able to maintain the feature.
I have not tested anything yet.
| void preciceAdapter::CouplingDataUser::writeGradients(std::vector<double>& gradientBuffer, const unsigned int dim) | ||
| { | ||
| adapterInfo("Data \"" + getDataName() + "\" does not support writing gradients. Please select a different " | ||
| "data or a different mapping configuration, which does not require " | ||
| "additional gradient information.", | ||
| "error"); | ||
| } |
There was a problem hiding this comment.
Nice way to default to a "not implemented" error message!
Only typo: change "a data" to just "data" or make it also "a different data configuration". ("data" is not countable)
| void preciceAdapter::CHT::Temperature::writeGradients(std::vector<double>& gradientBuffer, const unsigned int dim) | ||
| { | ||
| for (const label patchID : patchIDs_) | ||
| { | ||
| const auto& TGrad = fvc::grad(*T_); | ||
| forAll(TGrad().boundaryField()[patchID], i) | ||
| { | ||
| for (unsigned int d = 0; d < dim; ++d) | ||
| { | ||
| gradientBuffer[i * dim + d] = TGrad().boundaryField()[patchID][i][d]; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This essentially addresses the same problem as @thesamriel implemented in FF/TemperatureGradient.C in https://github.com/precice/openfoam-adapter/pull/281/files#diff-90c837067b5e748c5bc1af9d24a5ab6f0a5f7801bd91685900c27491c259a830
The main difference is that:
- This PR gives the gradient of (temperature) to preCICE, as gradient data
- Updates to the FF module #281 gives the (gradient of temperature) to preCICE, as normal data values
Would these two PRs need to be aligned somehow? @thesamriel I assume that this still cannot be shaped as a replacement for the respective addition of #281.
| if (precice_.isGradientDataRequired(couplingDataWriter->dataID())) | ||
| { | ||
| couplingDataWriter->writeGradients(gradientBuffer_, dim_); | ||
| precice_.writeBlockVectorGradientData(couplingDataWriter->dataID(), | ||
| numDataLocations_, | ||
| vertexIDs_, | ||
| gradientBuffer_.data()); |
There was a problem hiding this comment.
One example for vector data would be velocity. We should also implement that (in this or in a follow-up PR), otherwise this case will be completely unused for now.
|
Regarding the "avoiding code duplication": We now have a generic reader/writer (implemented by @vidulejs in #346). We also added some new features (e.g., new adapter config schema configuration) only to that, and in v2 we would make that (with a better architecture and naming) as the default, still allowing special case implementations, for anything that we cannot handle just with an Therefore, if this PR is still relevant, we could directly port it to the generic module (anytime) and forget about all other fields (Temperature is already supported). The user would then need to decide if the operation makes sense. |
2 participants
... in case gradient data is required for the mapping.
The current changes only implement the functionality for a simple CHT case. We need to go through all relevant fields and have a look, where it makes sense to support this feature. We also need to find a proper solution in order to avoid code duplication in all the fields, as the 'write' step looks similar for all fields (mainly depending on the number of data components).
TODO list:
docs/changelog-entries/(create directory if missing)