Publish the afmotion image and pull from docker hub, and add failure reporting - #95
Merged
avhekkink merged 7 commits intoSep 3, 2026
Conversation
…off of local docker image. Will push image to docker hub in the next phase
The plugin used the bare image name "afmotion". Docker resolves that name only against a local image, so the pipeline ran on one machine. The image is now on Docker Hub as cemrg/afmotion:1.0. It is the September 2024 build, published without a change: manifest digest sha256:6574a5bc4f79bb221ac5bd1c8cd3132abf05b626f4c04c6d444c3c1c64ade70c, 7,224,854,415 bytes, 22 layers. SetDockerImageAfmotion names the tag, and takes a tag argument for a later build. DockerAtrialStrainMotion reads the name from GetDockerImage(), so the image appears once. The mount and the positional host path do not change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DeTRpUD2TQPdHo5zWxwKg
avhekkink
marked this pull request as ready for review
September 1, 2026 11:53
alonsoJASL
reviewed
Sep 2, 2026
alonsoJASL
left a comment
Collaborator
There was a problem hiding this comment.
Just help me with that one comment on the line in the plugin's cpp:
class CemrgCommandLinel
Otherwise it's ready.
Step 10 ran two openCARP Laplace solves and reported success without doing them. Step 11 then failed, because the field step 10 must write was absent. Three defects composed. Pin the openCARP image to v19-0. Release v19.0 is the last one that declares ellip_use_pt and parab_use_pt. The latest tag is a development build from after their removal, and it rejects both parameters. Skip a PETSc option file that is not in the mount. openCARP stops with a segmentation fault when it cannot read an option file. Without the flag it uses its own solver settings, which match the settings in amg_cg_opts. Name phie.igb in OpenCarpDocker, and check the fields in UAC_Stage1 and UAC_Stage2. The check tested the output directory, which OpenCarpDocker creates itself, so every openCARP failure reported as a success. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013cT5BGXcsgwFczMGccPwsL
…on' into plugin/atrialstrainmotion-fix-step-10-and-11-UAC
…tion-fix-step-10-and-11-UAC Pin openCARP image to v19.0 and add PETSc option file fallback to unblock steps 10 and 11
avhekkink
deleted the
plugin/atrialstrainmotion-update-DockerAtrialStrainMotion
branch
September 3, 2026 10:43
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Published the afmotion image as cemrg/afmotion:1.0 on Docker Hub and pointed the code at it. Also added an output to check for every step so that a step no longer falsely reports success when nothing has been written.
Additions
RunAndCheckDockerAtrialStrainMotionSubcommandruns one afmotion subcommand and checks it.It replaces the per-step
ExecuteTouchandDockerAtrialStrainMotionpairs, and names the stepand the missing file in a warning dialogue.
SetDockerImageAfmotionnamescemrg/afmotion:1.0.afmotionwas a bare local image name,so the pipeline ran on one machine. The image is now on Docker Hub, public — the September 2024
build published without a change, digest
sha256:6574a5bc…ade70c, 22 layers. A rebuild needs aVTK 9.3 source tree, and the tag the author used isn't recorded anywhere. Will look into this as a separate piece of work.
Edits
DockerAtrialStrainMotionreturns a verdict. It takesexpectedOutputinstead of returningvoid, runs the container as the host user, and reads the image name fromGetDockerImage.GetDockerArgumentsnow passes--workdir=/data, and this repairs UAC Stage 1. It passed noworking directory before, so each container used the one it declares.
cemrg/uacdeclaresWorkingDir=/code, which root owns. Once PR Run the Docker containers as the host user #94 added--user, the host user could no longerwrite there, and
read_array_fiberwrites a temporary file by a relative path — so Stage 1 raisedPermissionErrorand died before it wroteLSbc1.vtx. The container exited 0 regardless, so step10 reported four missing outputs and no reason.
--workdir=/datapoints the working directory atthe mount, which is what
GetOpenCarpDockerCoreArgumentsalready does with--workdir=/shared.Nine
ExecuteTouchcalls are gone, acrossAutoLandMark,UAC_Stage2,CreateModel,Registration,GenerateCellAreaStrainsandJacobianThreshold. A pre-created empty file madethe check pass on a step that wrote nothing;
--userremoves the reason those calls existed.CreateModelandRegistrationstop at the first failure. Each subcommand reads what the onebefore it wrote. Two checks can still pass on a partial run —
generateCellAreaStrainsandcalcFiberStrainsloop in bash with noset -e— and a comment marks that limit.Impacted areas
AtrialStrainMotionView.{h,cpp}CemrgCommandLine.{h,cpp}GetDockerArgumentshas four callers. Three runcemrg/uacand all gain the working-directory fix;DockerCctaMultilabelSegmentationrunscemrg/ccta, which already declaresWorkingDir=/data, sonothing changes there. The Atrial Fibres plugin calls two of the three, so it had the same UAC
Stage 1 failure and gets the same fix. It is not re-run on this branch.
Testing
Verified on
V-0004and on the 2024V-0005regression fixture:V-0004. UAC Stage 1 now writes its five.vtxfiles, owned by the hostuser — it failed before the working-directory fix.
afmotion:latestremoved first soonly
cemrg/afmotion:1.0could serve it.autoLMreproduces the 2024 reference after a freshdocker pull, md514a5ed485da22b3e6a20009a26db8c54.MitkCemrgAppModulecompiles and links.Not verified:
ellip_use_ptandparab_use_pt.docker.opencarp.org/opencarp/opencarp:latestis a development build, andopenCARP removed both parameters after release
v19.0. The same command fails as root.GetDockerArgumentschange.GetDockerUserArgumentsreturns nothing there andExecuteTouchwas already a no-op, sothe checks behave differently by platform.