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

Rewrite mapping docs #308

Merged
merged 10 commits into from
Nov 30, 2023
Merged

Rewrite mapping docs #308

merged 10 commits into from
Nov 30, 2023

Conversation

davidscn
Copy link
Member

...according to preCICE v3

Closes precice/precice#1608

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.

Nice job. Minor tweaks below.

@MakisH For a feature that has undergone such dramatic changes in configuration, should we add a version box here mentioning some of the old configurations and pointing to the v2 pdf?

Copy link
Member

Choose a reason for hiding this comment

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

Why is "iterative solving" and "direct solving" in v3? Since there were GPU updates? Is this clear?

Copy link
Member

Choose a reason for hiding this comment

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

Without having looked at the text yet, what do the versions mean? When the feature was intoduced? But then, the (v3.0.0) is not clear to me in the iterative and direct. What is the meaning of the diamond?

Looking at the more detailed figure, I understand you want to say that some features are supported since v3. Why not have a complete figure, instead? This would give a good overview and be clearer.

The arrows probably don't mean anything here. I would rather use simple lines and start from "Mapping methods".

If you want to include GPUs in the picture, you can do that with additional visual elements (e.g., a colored dot next to each mapping).

Why are some circles filled and some not?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why is "iterative solving" and "direct solving" in v3? Since there were GPU updates? Is this clear?

I'm not sure I understand what you mean. Why do I have to give a reason for how we configure? There is a lengthy discussion about the rationale in a precice issue and it is independent of the GPU. The answer would probably be: because it resolves the configuration complexity of configuring all available options in the best way we could think of.

Why not have a complete figure, instead?

Putting the GPU executors here is not a good idea. It's more of an advanced topic and I would keep it as such. Considering that the whole docs is only valid for v3, I will remove the version hints and coloring

Copy link
Member

Choose a reason for hiding this comment

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

I also meant:

But then, the (v3.0.0) is not clear to me in the iterative and direct. What is the meaning of the diamond?

Copy link
Member Author

Choose a reason for hiding this comment

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

What is the meaning of the diamond?

methods are indicated by a circle. The 'how we solve' are not methods on it's own, so I tried to differentiate.

pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
@MakisH
Copy link
Member

MakisH commented Nov 22, 2023

@MakisH For a feature that has undergone such dramatic changes in configuration, should we add a version box here mentioning some of the old configurations and pointing to the v2 pdf?

I think that, when we release v3, we should point to the v2 PDF in the banner for a while, to make it visible. We could then also add a "Tip" alert box in the mapping config to point to the v2 PDF: https://precice.org/docs-meta-cheatsheet.html

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

Good overall, it definitely helps getting a better overview of the zoo of features.

I would like to read it again once all the current suggestions have been considered.

Copy link
Member

Choose a reason for hiding this comment

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

Without having looked at the text yet, what do the versions mean? When the feature was intoduced? But then, the (v3.0.0) is not clear to me in the iterative and direct. What is the meaning of the diamond?

Looking at the more detailed figure, I understand you want to say that some features are supported since v3. Why not have a complete figure, instead? This would give a good overview and be clearer.

The arrows probably don't mean anything here. I would rather use simple lines and start from "Mapping methods".

If you want to include GPUs in the picture, you can do that with additional visual elements (e.g., a colored dot next to each mapping).

Why are some circles filled and some not?

Copy link
Member

Choose a reason for hiding this comment

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

Why are both "iterative solving" and "direct solving" point to the same GPU bubble?
Shouldn't it be two bubbles with duplicate content? Aren't these two different mapping implementations?

Copy link
Member Author

Choose a reason for hiding this comment

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

The figure shows the executor, which is the same, and not the implementation

pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
Co-authored-by: Benjamin Uekermann <benjamin.uekermann@gmail.com>
@davidscn
Copy link
Member Author

I think that, when we release v3, we should point to the v2 PDF in the banner for a while, to make it visible. We could then also add a "Tip" alert box in the mapping config to point to the v2 PDF:

I added now an info box

Copy link
Member

@MakisH MakisH 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 overall, but I think it is a good time to process the whole page with LanguageTool. No further review iterations are needed from my side, looks clear. 👍

pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
pages/docs/configuration/configuration-mapping.md Outdated Show resolved Hide resolved
davidscn and others added 2 commits November 30, 2023 11:09
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
@davidscn davidscn merged commit 5de6273 into precice-v3 Nov 30, 2023
1 check passed
@davidscn
Copy link
Member Author

Thanks for the useful review comments! I will merge here for now, although the image has still an open discussion. We can still improve later on, I think.

@davidscn davidscn deleted the rewrite-mapping-docs branch November 30, 2023 10:19
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants