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

Update documentation for initialize #186

Merged
merged 20 commits into from
Jun 1, 2023

Conversation

BenjaminRodenberg
Copy link
Member

@BenjaminRodenberg BenjaminRodenberg commented Jul 16, 2022

Follows changes in implementation introduced in precice/precice#1368.

Todo

@BenjaminRodenberg BenjaminRodenberg added the breaking change A change will break backwards compatibilty label Jul 16, 2022
@BenjaminRodenberg BenjaminRodenberg self-assigned this Jul 16, 2022
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.

I am not sure yet, what the best option for the page couple-your-code-initializing-coupling-data is.

Options:

  • Explain both variants (old and new)
  • Remove page completely
  • Explain only new version (but is there much to explain?)

We should also write a porting guide, to which we should link from here.

…ling-data.md

Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>
@BenjaminRodenberg
Copy link
Member Author

BenjaminRodenberg commented Jul 18, 2022

I am not sure yet, what the best option for the page couple-your-code-initializing-coupling-data is.

Options:

* Explain both variants (old and new)

I would prefer only new, because this is less confusing and less work. If somebody is interested in the old version, one can refer to the old version of the docs.

* Remove page completely

I would keep it (see below).

* Explain only new version (but is there much to explain?)

There are some features that need explanation: 1) we need a new action. 2) we need to write data. 3) we have to specify this in the configuration. I'm not really sure where we could explain this instead without making another step in the step-by-step guide too complicated. And: data initialization is important. I think we should also give it some visibility.

We should also write a porting guide, to which we should link from here.

Agree. I'm currently a bit short on time. I guess the old porting guide (see https://precice.org/couple-your-code-porting-adapters.html) will act as a template here. Quick and dirty porting guide from my side:

  1. move initialize to the place where initializeData is called.
  2. remove calls of initializeData.

The diff of the tests in https://github.com/precice/precice/pull/1350/files#diff-4032663f1dad054c37ca6b17856ee821140a58a05378aae3e219adf266436be6 shows this workflow quite nicely.

@BenjaminRodenberg
Copy link
Member Author

This PR has been lying around for quite long. @uekerman I would suggest to merge it. We can still improve it later depending on user feedback, but I think the most important changes to keep the documentation up-to-date with version 3 are here.

Note: Porting guide has been extended already a while ago.

@uekerman
Copy link
Member

I am not sure we should merge already. So far, we did not break the documentation already. I would actually only break it once we release v3.

@BenjaminRodenberg BenjaminRodenberg changed the base branch from master to develop-v3 April 21, 2023 07:55
@BenjaminRodenberg
Copy link
Member Author

@uekerman should be ready to merge now that precice-v3 exists. Right?

@BenjaminRodenberg BenjaminRodenberg added this to the Version 3.0.0 milestone Apr 21, 2023
@BenjaminRodenberg
Copy link
Member Author

I also updated the initialization docs here w.r.t the removal of mesh and data IDs.

Copy link
Member Author

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are currently missing documentation updates for changes this PR depends on. I'm providing a review to give an overview:

Comment on lines -29 to -32
int displID = precice.getDataID("Displacements", meshID);
int forceID = precice.getDataID("Forces", meshID);
double* forces = new double[vertexSize*dim];
double* displacements = new double[vertexSize*dim];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removal does not really belong to this documentation update, because it is triggered by the update of mesh and data ids. But I did it here anyway.


```cpp
void initializeData();
bool requiresInitialData()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removal does not really belong to this documentation update, because it is triggered by the update of the action interface. But I did it here anyway.

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.

Sry for playing this back another time.

_includes/xmlreference.md Outdated Show resolved Hide resolved
Comment on lines 40 to 44
If you are using a serial coupling scheme, only initial data that is written from the second to the first participant is actually used. Initial data written from the first to the second participant will be ignored, because in a serial coupling scheme the first participant already computes its first result before the second participant even starts.

{% note %}
preCICE still supports data initialization for both participants, even if a serial coupling scheme is used. You can read our section on [time interpolation](couple-your-code-waveform) to learn more about the reasons.
{% endnote %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to read this part three times before understanding it. Could we improve it?
Instead of "written from A to B", I would always say "written to A and then communicated by preCICE from A to B".
Either all in a note or non. Remember that waveform will be the norm with v3 eventually.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No so sure about this whole paragraph anymore, because it makes things more complicated and actually we are now planning to use initial data for every participant for every coupling scheme. Also the second participant in a serial coupling scheme uses the initial data it receives from the first. Even for waveform order = 0 we use the initial data for the relative time = 0. This is a very narrow edge case, but it leads to a consistent treatment across windows. See also the following figure:

Screenshot from 2023-05-28 14-11-10

How about removing this paragraph and opening the whole topic later when we introduce time interpolation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also use the figure from above later in the waveform relaxation section, if it is useful. For the section on data initialization, I think it contains too much information that has not been introduced yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See ec217b5 for some steps into this direction.

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.

Let's try to move forward here. We, anyway, should have a closer look at all of these pages before releasing.
Please remove the edits in the xmlreference and merge.

_includes/xmlreference.md Outdated Show resolved Hide resolved
@BenjaminRodenberg BenjaminRodenberg merged commit 480ce67 into precice-v3 Jun 1, 2023
@BenjaminRodenberg BenjaminRodenberg deleted the initialize-data-v3.0.0 branch June 1, 2023 14:07
uekerman added a commit that referenced this pull request Feb 12, 2024
* Fix whitespace.

* Update action config docs to v3 (#254)

* Update Roadmap

* Update fundamentals-roadmap.md

* Use time step, not timestep. (#259)

* v3: Update documentation w.r.t getMaxTimeStep(). (#258)

* Update documentation w.r.t getMaxTimeStep().
* Implements changes from precice/precice#1623

---------

Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>

* Minor follow-up for #258.

* Update documentation for initialize (#186)

* Documentation for data initialization changes introduced in v3.0.0.
* Cleanup initializeData.

---------

Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>
Co-authored-by: Benjamin Uekermann <benjamin.uekermann@ipvs.uni-stuttgart.de>

* Update profiling section (#266)

* Update profiling section

* Update examples in profiling section

* Apply suggestions from code review

Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>

* Add motivation for custom tool

* Wording tweaks

* More rewording

* Update event files in output files

---------

Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>

* Fix typo in profiling

* Update profiling info in guide

* Update CMake variables to new names (#289)

* Clarify some instructions in porting guide (#287)

Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>

* Fix minor issues in tooling/performance-analysis (#272)

* Rename events to profiling

according to precice/precice #1787

* Mention where to find the precice-profiling

* Add hint regarding the event flag in precice-profiling

* Add recommendation for `imvj-restart-mode` in configuration of acceleration (#296)

* Update configuration-acceleration.md

Add recommendation to use restart mode in `IQN-IMVJ` method for large-scale simulations.

* Update pages/docs/configuration/configuration-acceleration.md

Improve descriptive accuracy

Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>

---------

Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>

* Apply trivial renaming changes.

* Fix naming.

* Add note.

* Update breaking API changes (#299)

---------

Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>

* Fix token

* Fix arg

* Bump preCICE version

* Update hint on formatting configuration files

* Refer to precice.hpp instead of participant.hpp

* Update version in docs sidebar

* Rewrite mapping docs (#308)

* Add overview figure

* Work in progress

* Fix links

* Transform pdfs to svgs

* Update kernel method section

* Also write about GPU executors

* Language edits by Benjamin and Makis

Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>

* Review updates

* Makis language edits

Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>

* Fix typos via aspell

---------

Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>

* Fix some outdated solver-interface and dimensions (#314)

* Migrate use-mesh to provide/receive-mesh

* Migrate binprecice to precice-tools

* Update config-visualizer page

* Include instructions to remove .data() in readData function (#313)

* Include instructions to remove .data() in readData function
* Mention span replacing raw pointers
* Remove raw pointers from general porting example

* Cleanup porting guide example

* Update couple your code

* Remove obsolete section regarding IDs

* Apply suggestions from code review

Co-authored-by: Ishaan Desai <ishaandesai@gmail.com>

* This PR adds C++ STL linking from Doxygen to cppreference

* Fix WD

* Link to porting guides from the source docs

* Add list of porting guides to overview

* Add new solid solver to Nutils adapter overview page (#325)

* Geometric Multiscale Documentation (#205)

Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>

* Update submodules

* Ignore tooling in images

* Update xmlreference to v3.0.0

* Cleanup porting guide API+Config (#317)

Co-authored-by: Ishaan Desai <ishaandesai@gmail.com>
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>

* Update the rust bindings page

* Fix link

* Make dt in readData mandatory in documentation (#257)

* v3: Make dt mandatory in documentation

* Update read data functions to use relativeReadTime.

* change precice_dt to preciceDt and solver_dt to solverDt to follow
conventions.

* Add missing.

* Shorten a bit.

* Minor follow-up for #258.

* Add figure.

* Remove outdated note.

* Use dt properly.

* Remove unnecessary pdf.

* Partial update of documentation w.r.t breaking changes.

* Redice diff.

* Redice diff.

* Reduce diff.

* Fix some more breaking changes.

* Add how dt is computed. See #257.

* Apply suggestions from code review

Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>

* Remove unneeded.

* Add pointer to interpolation section.

* Fix format.

* Add advice on precice/precice#1866.

* Apply suggestions from code review

* Update some minor, obviously outdated points.

* Update png.

* Update pages/docs/couple-your-code/couple-your-code-mesh-and-data-access.md

Co-authored-by: Frédéric Simonis <simonisfrederic@gmail.com>

* Update heading.

* Moved changes to #326.

* Update pages/docs/couple-your-code/couple-your-code-mesh-and-data-access.md

* Update pages/docs/couple-your-code/couple-your-code-waveform.md

---------

Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
Co-authored-by: Frédéric Simonis <simonisfrederic@gmail.com>
Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>

* Update submodules

* Remove v3 tip from roadmap

* Add rust bindings to installation overview

* Update libprecice2 to libprecice3 on Ubuntu packages page

* Update header names in installation-linking

* Update porting information on configuration overview page

* Update several things on configuration basics page

* Update configuration communication page

* Revert tweaks in configuration basics page

Previously, I changed from precice to participant as name for the preCICE handle and I used "Force" instead of "Forces" to be consistent with the tutorial guidelines. But both changes would also need updates on many other pages. Thus, I left them for the moment for consistency.

* Refer to QN default values on acceleration configuration

* Update porting info and mention Rust on couple your code start

* Change global dim to mesh dim in step 3

* Remove dim handling from step 6

* Fix getDataDim arguments in step 3

* Fix dim handling in step 9

* Fix dim in direct access

* Fix and tweak porting guide

* Make linter happy

* Make linter happier

* Add missing tutorials in the sidebar and overview (#334)

* Add partitioned-pipe-two-phase to sidebar and overview

* Add oscillator overlap in sidebar and overview

* Add partitioned-heat-conduction-overlap to the sidebar and overview

* Add volume-coupled-flow to sidebar and overview

* Add two-scale-heat-conduction to sidebar and overview

* Update pages/tutorials/tutorials.md

Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>

* Update pages/tutorials/tutorials.md

---------

Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>

* Update bindings overview picture (#333)

* Update bindings overview picture

* Move arrow of Python bindings to precice.hpp

* Add Rust bindings

---------

Co-authored-by: Benjamin Rodenberg <benjamin.rodenberg@in.tum.de>
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
Co-authored-by: Benjamin Rodenberg <benjamin.rodenberg@cit.tum.de>
Co-authored-by: Frédéric Simonis <simonisfrederic@gmail.com>
Co-authored-by: David Schneider <david.schneider@ipvs.uni-stuttgart.de>
Co-authored-by: carme-hp <71499004+carme-hp@users.noreply.github.com>
Co-authored-by: June <94080048+Fujikawas@users.noreply.github.com>
Co-authored-by: homspons <ge35joj>
Co-authored-by: Ishaan Desai <ishaandesai@gmail.com>
Co-authored-by: Elia Zonta <99761626+ezonta@users.noreply.github.com>
Co-authored-by: precice-bot <info@precice.org>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change A change will break backwards compatibilty
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants