systemtests: add heat-exchanger-simplified test suite by AdityaGupta716 · Pull Request #731 · precice/tutorials

AdityaGupta716 · 2026-02-28T14:41:28Z

What this PR does

Adds heat-exchanger-simplified to the system test suites in tools/tests/tests.yaml as both a new standalone suite heat_exchanger_simplified_test and as an entry in the existing release_test suite.

Also fixes several Docker and Ubuntu 24.04 compatibility issues that prevented local system test runs.

Changes

tests.yaml

Added heat_exchanger_simplified_test standalone suite with case (fluid-top-openfoam, fluid-btm-openfoam, solid-calculix)
Added heat-exchanger-simplified to release_test so it runs in release CI

Systemtest.py

Added _get_docker_compose_cmd() to auto-detect docker compose vs docker-compose
Replaced all 3 hardcoded docker compose --file Popen calls to use the detection helper with -f

Dockerfile (ubuntu_2404)

Used userdel -r ubuntu before creating precice user, removing the default Ubuntu 24.04 user including its home directory — cleaner fix for the UID/GID 1000 conflict
Added =master defaults for all six adapter ARGs: OPENFOAM_ADAPTER_REF, PYTHON_BINDINGS_REF, FENICS_ADAPTER_REF, CALCULIX_ADAPTER_REF, SU2_ADAPTER_REF, DEALII_ADAPTER_REF
Added --tries=3 --retry-connrefused --timeout=30 to wget for both CalculiX and SU2 downloads

components.yaml

Added PYTHON_BINDINGS_REF to calculix-adapter build args

Verification

Confirmed heat-exchanger-simplified has a complete metadata.yaml with all required participants and cases
Verified case names match exactly with metadata.yaml
Confirmed correct parsing via python3 print_test_suites.py — both heat_exchanger_simplified_test and release_test show the new tutorial
Ran python3 systemtests.py --suites=heat_exchanger_simplified_test --build_args=TUTORIALS_REF:develop — Docker images build and coupled run completes; fails only at field-comparison due to missing reference archive (expected)

Notes on reference results

Reference results for heat-exchanger-simplified will need to be generated on a Linux machine via CI or generate_reference_results.py. The .gitkeep placeholder has been added to heat-exchanger-simplified/reference-results/.

Checklist

I added a summary of any user-facing changes in changelog-entries/731.md
I will remember to squash-and-merge with a useful summary

precice-bot · 2026-02-28T14:53:44Z

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

https://precice.discourse.group/t/gsoc-2026-aditya-gupta/2773/2

AdityaGupta716 · 2026-03-16T21:46:53Z

@MakisH plz review!
While adding the test suite I ran into a few things locally Ubuntu 24.04 has a default ubuntu user with UID/GID 1000 which breaks the container build, the Systemtest dataclass was referencing self.timeout without ever declaring the field (would crash with AttributeError), and the CalculiX/SU2 downloads occasionally fail without a read timeout. Fixed all of those along the way, plus replaced the global docker compose detection with lru_cache and separated build vs solver timeouts.
Tested on macOS build, 3-participant solver run (500 timesteps), and field comparison all passed.
Reference results are generated (24MB). LFS pointer is committed but the binary needs uploading to the preCICE LFS server , happy to share the file directly if needed.

MakisH

Thanks for the detailed PR. Some interesting ideas here, but a PR implementing so many ideas at once cannot really be reviewed/merged. I would suggest restricting it to the most important ones that make sense to include as one PR, and delegate the rest to issues and other PRs.

Fine to keep this open for now to discuss which of these ideas would make sense to port to other PRs and how.

MakisH · 2026-03-26T08:23:50Z

changelog-entries/731.md

This is difficult to review as-is, because of the many changes in one PR, which is also clear from the five entries in this file. I would split this into one PR per work package. All of them sound useful, but should not be in one PR.

I've stripped the PR down accordingly , All other changes will be moved to separate issues/PRs.

MakisH · 2026-03-26T08:24:14Z

heat-exchanger-simplified/reference-results/.gitkeep

This file is not needed.

MakisH · 2026-03-26T08:26:46Z

...ger-simplified/reference-results/fluid-top-openfoam_fluid-btm-openfoam_solid-calculix.tar.gz

The reference results are not meant to be added by the contributor - we have a GitHub Actions workflow for that, which maintainers can trigger. This is also important since we compare floating-point values, which might change based on the system.

Removed, will let the CI workflow handle reference result generation.

MakisH · 2026-03-26T08:28:02Z

tools/tests/dockerfiles/ubuntu_2404/Dockerfile

+# Ubuntu 24.04+ images include a default user "ubuntu" with UID/GID 1000; remove it so we can create "precice" with the same UID/GID.
+RUN userdel ubuntu 2>/dev/null || true; \
+    groupdel ubuntu 2>/dev/null || true; \
+    groupadd -g ${PRECICE_GID} precice && useradd -u ${PRECICE_UID} -g ${PRECICE_GID} -ms /bin/bash precice


Interesting, but I would like to see some evidence/reference for the issue, or test it separately. A smaller PR just for this would be good.

MakisH · 2026-03-26T08:35:39Z

tools/tests/dockerfiles/ubuntu_2404/Dockerfile

 #Download Calculix
 WORKDIR /home/precice
-RUN wget http://www.dhondt.de/ccx_${CALCULIX_VERSION}.src.tar.bz2 && \
+RUN wget --tries=3 --retry-connrefused --timeout=30 --read-timeout=120 http://www.dhondt.de/ccx_${CALCULIX_VERSION}.src.tar.bz2 && \


A --tries setting would also be useful for the wget of OpenFOAM. That one actually fails often.

The --retry-connrefused should not be needed: what should change if connection is refused? We are using publicly accessible files, one refuse should mean that the page has been moved.

What are you trying to achieve with the timeout? Increase it or decrease it? Why? Default value seems to be 900s.

timeout=30 sets the connect timeout to 30s (to fail fast if the server is unreachable), while --read-timeout=120 gives 2 minutes for slow downloads to complete. The intent was to avoid the 900s default hanging the build for 15 minutes on a dead connection, while still allowing time for large files to transfer. You're right that --retry-connrefused doesn't make sense for public files will drop it. should i make the changes in this pr or open a seperate one ?

MakisH · 2026-03-26T08:38:56Z

tools/tests/systemtests/Systemtest.py

+BUILD_TIMEOUT = 1800  # builds include compilation + downloads; increased from upstream 900s to prevent timeouts on slow machines or networks
+GLOBAL_TIMEOUT = 900   # timeout for solver runs and field comparison


Instead of increasing it globally (which would make the system tests on GitHub Actions exit later if something goes wrong), a better approach to deal with slower local builds would be a way to override this with an environment variable.

Removed from this PR. Will address in #735 keeping GLOBAL_TIMEOUT as is (no rename), making it overridable via PRECICE_SYSTEMTEST_TIMEOUT environment variable as you suggested. The per-test timeout field in tests.yaml will default to GLOBAL_TIMEOUT instead of hardcoding a separate value.

MakisH · 2026-03-26T08:49:38Z

tools/tests/systemtests/Systemtest.py

+@functools.lru_cache(maxsize=1)
+def _get_docker_compose_cmd():
+    """Return the docker compose command list: ['docker', 'compose'] or ['docker-compose']."""
+    try:
+        subprocess.run(
+            ['docker', 'compose', 'version'],
+            capture_output=True, timeout=5, check=True,
+        )
+        return ['docker', 'compose']
+    except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired):
+        try:
+            subprocess.run(
+                ['docker-compose', 'version'],
+                capture_output=True, timeout=5, check=True,
+            )
+            return ['docker-compose']
+        except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired):
+            logging.warning("Could not detect 'docker compose' or 'docker-compose'; defaulting to 'docker compose'. This will fail with a clear error if Docker is not available.")
+            return ['docker', 'compose']
+
+


I would not add this, to keep the code shorter and simpler. Clearly documenting the requirement should be enough - this script is mainly meant to be used by specific users that first read the documentation or talk to someone.

I would rather have the script fail and then have the person talk to a maintainer, than automatically switching what it runs. That could hide issues down the line.

The official way nowadays is docker compose. We should just use that everywhere, as the script currently does. If that is not available, the solution should be to upgrade Docker.

Agreed Better to let it fail so the user knows to upgrade Docker.Removed it

MakisH · 2026-03-26T08:54:33Z

tools/tests/systemtests/Systemtest.py

+            # On macOS, GID may conflict with Ubuntu system groups (e.g. GID 20 = dialout).
+            # Fall back to 1000 if the GID is below 100 (system range in Ubuntu).
+            if gid < 100:
+                logging.warning(f"Host GID {gid} is in Ubuntu system range, using 1000 instead.")
+                gid = 1000


I am not sure that I understand the issue, it should be in a separate PR/issue, with references on what the group numbers are in the related systems.

It's true that we have not tested this with macOS, so such issues would be useful, for developers that develop on macOS (we don't really have much experience there yet among maintainers).

will do , removed it from this pr

MakisH · 2026-03-26T08:55:56Z

tools/tests/components.yaml

      default: "master"
    PYTHON_BINDINGS_REF:
-      semnantic: Git ref of the Python bindings to use
+      description: Git ref of the Python bindings to use


Sounds like we don't even use this anywhere, so I would rather delete it (in another PR).

ok,removed it

MakisH · 2026-03-26T09:00:52Z

tools/tests/tests.yaml

+          - fluid-btm-openfoam
+          - solid-calculix
+        reference_result: ./heat-exchanger-simplified/reference-results/fluid-top-openfoam_fluid-btm-openfoam_solid-calculix.tar.gz
+  heat_exchanger_simplified_test:


I was not aware of anchors in YAML, that can indeed be useful here.

Interesting to have individual tutorials as test cases, but I think this should be a stand-alone PR to add different tutorials as test cases.

AdityaGupta716 · 2026-03-26T13:23:44Z

hi @MakisH thanks for the review i will check out everything and reply shortly currently a bit busy with viva in college will surely check out everything by tomorrow

MakisH · 2026-03-26T14:06:03Z

hi @MakisH thanks for the review i will check out everything and reply shortly currently a bit busy with viva in college will surely check out everything by tomorrow

No hurry, I am the bottleneck here. Good luck with your viva!

AdityaGupta716 mentioned this pull request Feb 28, 2026

Tests included in the release test suite #448

Open

25 tasks

MakisH added systemtests GSoC Contributed in the context of the Google Summer of Code labels Mar 1, 2026

AdityaGupta716 force-pushed the systemtests/add-heat-exchanger-simplified branch 5 times, most recently from e576056 to e0727cb Compare March 6, 2026 13:29

MakisH reviewed Mar 26, 2026

View reviewed changes

This was referenced Mar 26, 2026

systemtests: add configurable timeout per test case #735

Open

systemtests: add per-test max_time override for precice-config #744

Open

AdityaGupta716 force-pushed the systemtests/add-heat-exchanger-simplified branch from 43acd04 to 149f4f1 Compare March 26, 2026 13:08

AdityaGupta716 force-pushed the systemtests/add-heat-exchanger-simplified branch 2 times, most recently from 3539239 to 9205670 Compare March 26, 2026 21:07

systemtests: add heat-exchanger-simplified test suite

f4e5008

AdityaGupta716 force-pushed the systemtests/add-heat-exchanger-simplified branch from 9205670 to f4e5008 Compare March 26, 2026 23:15

		BUILD_TIMEOUT = 1800 # builds include compilation + downloads; increased from upstream 900s to prevent timeouts on slow machines or networks
		GLOBAL_TIMEOUT = 900 # timeout for solver runs and field comparison

Uh oh!

Conversation

AdityaGupta716 commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Changes

Verification

Notes on reference results

Checklist

Uh oh!

precice-bot commented Feb 28, 2026

Uh oh!

AdityaGupta716 commented Mar 16, 2026

Uh oh!

MakisH left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdityaGupta716 Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdityaGupta716 Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdityaGupta716 commented Mar 26, 2026

Uh oh!

MakisH commented Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AdityaGupta716 commented Feb 28, 2026 •

edited

Loading

AdityaGupta716 Mar 26, 2026 •

edited

Loading

AdityaGupta716 Mar 26, 2026 •

edited

Loading