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

Compile tentative adapter checklist #7

Open
uekerman opened this issue Mar 28, 2024 · 14 comments
Open

Compile tentative adapter checklist #7

uekerman opened this issue Mar 28, 2024 · 14 comments

Comments

@uekerman
Copy link
Member

The adapter checklist should contain criteria that an adapter needs to fulfill to be "preCICE conforming". If necessary, criteria could also be organized in hierarchical quality levels (bronze, silver, gold). We then want to establish a clear review workflow. Once conforming, adapters are listed on the preCICE website together with the necessary metadata and get a "preCICE badge". Both should be independent of where the actual source code is hosted. Adapters hosted under the preCICE organization on GitHub, on the other hand, need to fulfill a fixed quality level (e.g., at least "bronze"). Similarly, all adapters with a fixed quality level (e.g., at least "silver"), are bundled into the preCICE distribution. Together with the checklist, we also want to compile templates, for example for adapter documentation and automation (formatting, documentation hosting, building, running tests, etc.).

Besides standard FAIR4RS criteria, one could include the following:

  • adapter configuration, which adheres to a solver-independent standard (needs to be defined) if no
    solver-internal solution exists (such the OpenFOAM dictionary for the OpenFOAM adapter),
  • coupling data and meshes adhere to a standard naming pattern (needs to be defined)
  • defined (standard) versioning scheme, changelog, and clear information, which solver versions are
    supported
  • documentation (provide README.md template) on how to build/configure/cite/...
  • explanation of which preCICE features are supported or not supported (include potential list in
    README.md template)
  • defined code style and provided formatter
  • at least one provided application case to test the adapter
  • unit tests
  • configurable logging (use preCICE for logging through API?)
  • contribution guidelines and potentially pull request template
  • information on how the adapter was validated and how validation could be reproduced
  • contact information of original developers and current maintainers
  • citation of published overhead study
@uekerman uekerman converted this from a draft issue Mar 28, 2024
@fsimonis
Copy link
Member

fsimonis commented Apr 24, 2024

As @uekerman mentioned in precice/precice#1647, standardizing adapter profiling could also make sense.

defined code style and provided formatter

We have the precice-configuration-formatter, which is included in https://github.com/precice/precice-pre-commit-hooks/tree/main/format_precice_config
It may be worth moving that to a separate pypi package for easier integration.

configurable logging (use preCICE for logging through API?)

We did some groundwork regarding logging when working on the Fluent adapter precice/precice#1305 and when integrating the logging with ASTE precice/aste#73. It is currently possible to configure logging without an API in preCICE by going through the boost.log core.

@uekerman
Copy link
Member Author

Reasons why using preCICE for logging could still be advantageous:

  • Users that don't know any logging package don't need to learn one
  • Language-independent solution
  • Configuration of the logger (concerning all preCICE things) in single place

@fsimonis
Copy link
Member

fsimonis commented Apr 24, 2024

Possible API design

General:

  • I would avoid any formatting capability in the API and only receive a message to log.
  • We can still handle debug log in preCICE even if we use a Release build. Currently, we only turn the PRECICE_Debug() into a no-op.
  • Should error logs still result in a crash?

Function + Enum

Participant::log("component", Severity::Info, "message");

Only 1 function, but requires handling an enum in bindings.

Function for severities

Participant::logDebug(  "component", "message");
Participant::logInfo(   "component", "message");
Participant::logWarning("component", "message");
Participant::logError(  "component", "message");

Easier to handle in bindings, but many functions.

@uekerman
Copy link
Member Author

This is pretty much what I had in mind. I guess we could even remove component and set this always to "adapter".

@fsimonis
Copy link
Member

Specifying the component could make run scripts that start both solver easier to follow.
But that is as far as I can come up with use-cases.

@MakisH
Copy link
Member

MakisH commented May 2, 2024

I have the feeling this discussion should better be moved into a separate issue in precice/precice.

I like the logDebug, logInfo, etc. It should indeed be the most portable solution, and I don't think that the more functions is really an issue. These are just a few, very simple, very similar auxiliary functions.

I assume that, if preCICE is built without debug messages, but the adapter is built with debug messages enabled, the user should still see debug messages from the adapter, right? As an adapter developer, if I use preCICE built in release mode (e.g., on a cluster, installed by an admin), I would still like to see the debug messages I add in my adapter.

Specifying the component could make run scripts that start both solver easier to follow.

Do we still have such scripts anywhere? Users might, but we generally discourage it, no?

@fsimonis
Copy link
Member

fsimonis commented May 3, 2024

Do we still have such scripts anywhere? Users might, but we generally discourage it, no?

It can be convenient to quickly start both solvers. Especially when environment variables need to be set (including venv) or one is on a cluster. Realistically all of this should go into scripts though to make this easier to repeat.

@BenjaminRodenberg
Copy link
Member

I would suggest to require data initialization to reach a certain level, because not having initial data makes the tutorials less flexible and can lead to bugs in the time stepping (degradation of convergence order). See precice/precice#2033.

@uekerman
Copy link
Member Author

uekerman commented Jul 2, 2024

Outcome of the first discussion with @MakisH. Further discussion is planned for the coding days.

Main idea

What is an adapter? An adapter can be an independent code, but can also mean solver + integrated adapter. Adapter can be a class, a patch, a library, or something else. But it needs to be something > 0. The "Nutils adapter" in this sense is no adapter as it is only a collection of application cases. We could even go one step further and include tools as well, i.e. everything that calls preCICE, or even everything that concerns preCICE.

We distinguish between metadata and best practices. The latter has three level, i.e. bronze, silver, and gold. We list on the website every adapter that has all required metadata and at least bronze level. If an adapter has at least silver level, hosting under the preCICE GitHub organization is possible, but no must.

Metadata

The usual things. Non-complete list:

  • name of the solver
  • original developers
  • current maintainers + contact information
  • available versions
  • type of adapter (curated list: stand-alone, patch, integrated, ...)
  • repo url or how to access
  • how to cite
  • license

Best practices

Each level has a main theme to simplify orientation. The lists are not complete yet, but should rather give some first impression.

Bronze

Findable and Accessible, i.e., the adapter is working and documented. Others can find, build, and run the code and they should understand what is does. The bare minimum. We need a low entry barrier.

  • at least one provided application case to test the adapter (either through independent application case or included in adapter)
  • version control (open is optional)
  • (standard) versioning scheme
  • README (or similar place):
    • documented which solver versions are supported
    • how to build (incl. dependencies)
    • how to configure if available; always: what data is coupled / supported
    • how to cite
    • how to run application case
    • application background and/or nature of coupling (e.g. surface vs. volume coupling; does it support multiple patches?; transient or steady-state; ...)
    • explanation of which preCICE features are supported or not supported (e.g. implicit coupling, mesh connectivity, data initialization, subcycling, multiple meshes); tick from curated list
    • whether solver features are restricted (e.g. cannot be run in parallel or cannot use adaptive time stepping or cannot use higher-order elements)
  • comments and error messages, if available, are in English
  • needs to have a license

Silver

Interoperable and maintainable. The adapter plays well with other adapters from the community and feels part of the preCICE ecosystem. The preCICE maintainers are, if necessary, able to maintain the adapter (e.g. update it to newer preCICE versions).

  • adapter configuration covering usual things (mesh name, data names, participant name, configuration name)
    • and standard names need to be possible (e.g. Displacement)
  • defined code style and provided formatter
  • changelog
  • issue tracker
  • information on how the adapter was validated and how validation could be reproduced
  • documentation rendered, either:
    • on precice.org (i.e. in docs subfolder and multiple markdown files with specific headers)
    • somewhere external
  • repo public or at least preCICE team has access
  • configurable logging, meaning at least "no logging, release logging, and debug logging" (optional: use preCICE for logging through API)

Gold

Reusable, community-ready, and integrated. Others can easily reuse and extend the adapter for their own needs. Maintaining the adapter could be shared among many. We can integrate the adapter and corresponding application cases into our development workflows and make sure that we will not break it. All tooling works seamlessly.

  • contribution guidelines and potentially pull request template
  • configuration follows schema
  • unit tests (using the fake preCICE implementation)
  • system-tests ready
  • peer-reviewed paper with validation
  • documentation on how to extend the adapter (i.e. docs about the architecture)
  • packaged (if there is no "better" place (e.g. PyPI), at least Spack)
    • that means implicitly that the solver itself needs to be packaged as well

@MakisH
Copy link
Member

MakisH commented Jul 11, 2024

Notes from the internal mini world-café:

Metadata

No related comments.

Bronze

  • We require a test case, but what should be the test coverage? Only a minimum set of features, or all features?
  • README: Rephrase the how to build (incl. dependencies) to clarify that we mean which dependencies are required, not how to build the dependencies.
  • README: How to run (the provided) application case overlaps with the pointer to a test case.
  • preCICE features that we could check for being supported:
    • 2D/3D meshes (which is the out-of-plane axis?)
    • How the dead-axis is handled for RBF mapping
    • Mesh connectivity
    • Implicit coupling
    • Timestep from first participant and essentially having dt = min(dt_solver, dt_precice)

Silver

  • Not requiring the repository to be public and open-source triggered complaints.
    • On the one hand, maintenance without having a public repository (with tests) is complicated.
    • On the other hand, we want to support open-source practices.
    • Bronze should be the maximum level allowing for non-public repositories.
    • Maybe we could make the public/private a separate tag/sublevel and not a requirement.
    • I add: We should clarify what are use cases for private (but available to some users) repositories and how they fit into what we want.

Gold

  • Requiring a paper to already be published means that an adapter cannot achieve Gold status before that.
    • Is this something we want?
    • I add: if one knows that they cannot achieve this, maybe they don't even start working on achieving Gold level.
  • On the other hand: Validation and reproducibility comes too late.
    • I add: Maybe we can break this into smaller achievements and have validation and reproducibility in every level? (basic/advanced/best validation and reproducibility). We already kind-of do that, we need to make it more explicit.

Other notes

  • To incentivize going up the ladder of quality levels, we should (as a community) integrate these levels into the funding process: a proposal should relate to these levels ("we want to achieve gold level") and a report/follow-up review should check whether this was achieved.
  • To track which features/adapters/application cases are used, we could give citation guidelines for specific features (idea from ESPResSo).
  • In the matadata, we could also integrate a way to declare yourself as user or interested in an adapter/application case(/feature). The technical implementation does not really matter. We could even add a Discourse tag for each adapter and track the number of topics/posts in the forum using this tag, as a rough metric. We already refer to these tags in publications.

@MakisH
Copy link
Member

MakisH commented Jul 12, 2024

If we also want to use Nix to prepare stacks with the distribution (e.g., the VM, the live USB, etc), then we could also require that, at the gold level, each adapter is available as a Nix package.

This would automatically disqualify any poorly maintained/exotic solvers, but could strengthen everything else.

@MakisH
Copy link
Member

MakisH commented Jul 12, 2024

  • at least one provided application case to test the adapter (either through independent application case or included in adapter)

We should probably explicitly name this "minimal working example", to signify that we are not looking for a realistic case, which would then be complicated to run on a laptop.

The question then is, what if the adapter has no MWE, but only a complicated published case.

@precice-bot
Copy link

This issue has been mentioned on preCICE Forum on Discourse. There might be relevant details there:

https://precice.discourse.group/t/shape-the-future-of-the-precice-ecosystem-the-preeco-project/2019/1

@MakisH
Copy link
Member

MakisH commented Jul 31, 2024

Next step: I will prepare a PR with such a checklist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: WP2 Compile standard (2024-2025)
Development

No branches or pull requests

5 participants