Conversation
|
The PETSc RBF mapping contains two uses of the data IDs to create some kind of cache key. @uekerman @davidscn Do you have an idea on how to get rid of this? precice/src/mapping/PetRadialBasisFctMapping.hpp Lines 661 to 664 in a7ac32c precice/src/mapping/PetRadialBasisFctMapping.hpp Lines 756 to 760 in a7ac32c |
This is for storing the start value for the iterative solver. Might also be useful for the Gingko CG. |
|
Turns out that we need to add dimensionality information to the Sample. |
|
If we want to keep this functionality, we could change the Mapping to: Then in the MappingContext: if (mapping.isIterative()) {
mapping.map(inData, outData, _lastSolution);
_lastSolution = outData;
} else {
mapping.map(inData, outData);
} |
Work on samples instead of data IDs Handle initial guesses of transient mappings in the data context
93c7e18 to
7eee01c
Compare
|
I added the dimensionality to time::Sample and migrated the mapping to it. I touched tests only to add the data dimensionality of samples. |
uekerman
left a comment
There was a problem hiding this comment.
Looks good, but I only had a quick look.
- @BenjaminRodenberg FYI changes in
Sampleetc - @davidscn FYI changes to mappings
- We need a changelog entry as we now treat the initial guesses differently.
- We need tests for the new solution caching.
|
When combining this with waveforms, is there a separate initial guess for each time step? Probably doesn't work as the time grid might be different in each iteration. |
Co-authored-by: Benjamin Uekermann <[email protected]>
There is one initial guess per data and mapping. The possibly differing timestamps of the stamples is a reason of why this is impractical to implement. |
|
MakisH
left a comment
There was a problem hiding this comment.
I skimmed through all files. The general need is kind of clear: we now have data on different times, so the mapping also needs to work using time samples. Don't rely on my review as an expert view, of course, but only as potentially useful details.
I am not sure I get the motivation and the need for the direct-iterative classification, though (see comment), without being able to suggest any better alternative.
Answering on some questions @fsimonis asked directly to me:
- Do the interfaces and assumptions make sense?: To the extent I understand how everything works together, yes. The interface does not look too different than before, it is mostly a matter of working types that change. I am a bit unsure about the
time::Sampleclass (see comment). - Can you see issues that I haven't thought about?: Not really.
- How does this behave in the geometric multi-scale situations?: There, we define new mappings, which should all be direct (if I understand the classification). Since there were very few changes in, e.g., the nearest-neighbor mapping, only few changes would be needed in the geometric multiscale as well. There seems to be a notion of dimensions everywhere, so I guess we can work around that. Maybe @ezonta could also comment here.
- Is there more information necessary?: I don't see anything right now, but I may need to look at it again once I understand the whole concept better.
- Should the return type be a
Sampleinstead of aVectorXDto make the data dimensions explicit?: Why is it currently like this? Wouldn't mapping from aSampleto aSample(or from aVectorXDto aVectorXD) be the default thought?
Co-authored-by: Gerasimos Chourdakis <[email protected]>
* Refactor data information out of mapping Work on samples instead of data IDs Handle initial guesses of transient mappings in the data context * Enable initial guess for mu * Remove mesh integral data versions * Assert that both input and output contain items * Port mapping in DataContext to samples * Add dimension to time::Sample * Replace mapping::Sample with time::Sample * Fix include * Add documentation to Sample * Extend documentation of Mapping * Fix log creation for precice without petsc * Directly map samples * Remove unused headers * Remove unpleasant default constructors * Rename and clarify the initial guess storage * Fix comment * Rename transient to iterative in Mapping * Improve the logging of mappings * Test different guesses * Add changelog * Fix Wording Co-authored-by: Benjamin Uekermann <[email protected]> * Reset initial guesses after the read mappings * Change wording Co-authored-by: Gerasimos Chourdakis <[email protected]> * Remove incorrect comment * Remove dead code comment * Improve initialGuess comment in PETSc Mapping * Extend Sample class description * Remove unused header * Rename dimension to dimensions in test * Explain direct mapping * Reword changelog * Unify comments * Rename parameter * Rename InitialGuessRequirement options * Renamed Mapping interface regarding isIterative mappings --------- Co-authored-by: Benjamin Uekermann <[email protected]> Co-authored-by: Gerasimos Chourdakis <[email protected]>
Main changes of this PR
This PR moves the Mapping to a sample-based interface and adds (real) support for transient mappings.
A sample combines data dimensionality, values and gradients.
General design is now for
map():SampleTransient mappings (additional constructor argument) now require to call the second variant of map:
SampleMotivation and additional information
DataContext now directly maps samples and creates now output samples.
DataContext also owns the initial guesses for transient mappings.
We reset initial guesses to zero after every time window. So mappings are transient during iterations.
Part of the migration to waveforms.
Closes #1651
PETRBF experiments
Based on v2.5.0, the perpendicular flap tutorial using calclix and nutils:
Tests are:
Conclusions:
Author's checklist
pre-commithook to prevent dirty commits and usedpre-commit run --allto format old commits.make changelogif there are user-observable changes since the last release.Reviewers' checklist