From 931b5a3c119f6e55c57b23fa068c944ad5977aa8 Mon Sep 17 00:00:00 2001 From: Derek Monroe <50217685+dcmonroe@users.noreply.github.com> Date: Sat, 28 Dec 2024 22:45:26 -0500 Subject: [PATCH 01/10] Update happy.py | mem check for singularity containers Running happy in singularity on a Linux cluster caused crashes despite ample allocated memory/cores. I sandboxed the .simg and rebulit locally to confirm that this fix works. --- rapidtide/workflows/happy.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/rapidtide/workflows/happy.py b/rapidtide/workflows/happy.py index ea414744..868b25c2 100644 --- a/rapidtide/workflows/happy.py +++ b/rapidtide/workflows/happy.py @@ -87,6 +87,17 @@ def happy_main(argparsingfunc): args.runningindocker = True args.dockermemfree, args.dockermemswap = tide_util.findavailablemem() tide_util.setmemlimit(args.dockermemfree) + + # if running Apptainer/Singularity, this is necessary to enforce mmemory limits properly + # otherwise likely to error out in gzip.py or at voxelnormalize step + try: + singularity_env = os.environ.get("SINGULARITY_NAME") or os.environ.get("SINGULARITY_CONTAINER") + except KeyError: + args.runninginsingularity = False + else: + args.runninginsingularity = True + args.singularitymemfree, args.singularitymemswap = tide_util.findavailablemem() + tide_util.setmemlimit(args.singularitymemfree) # Set up loggers for workflow setup_logger( From d41d3e29613fa98992ded878658457b1004a9dd1 Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 10:16:39 -0500 Subject: [PATCH 02/10] Update happy.py Disable this fix if running at CircleCI --- rapidtide/workflows/happy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rapidtide/workflows/happy.py b/rapidtide/workflows/happy.py index 514d41b7..5df9ff2e 100644 --- a/rapidtide/workflows/happy.py +++ b/rapidtide/workflows/happy.py @@ -91,7 +91,7 @@ def happy_main(argparsingfunc): # if running Apptainer/Singularity, this is necessary to enforce mmemory limits properly # otherwise likely to error out in gzip.py or at voxelnormalize step try: - singularity_env = os.environ.get("SINGULARITY_NAME") or os.environ.get("SINGULARITY_CONTAINER") + singularity_env = (os.environ.get("SINGULARITY_NAME") or os.environ.get("SINGULARITY_CONTAINER")) and not os.environ.get("CIRCLECI") except KeyError: args.runningincontainer = False else: From e66cd26fda41db4e897b2c4a61026f1c427fe16d Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 10:40:20 -0500 Subject: [PATCH 03/10] Update happy.py Made CircleCI check more sophisticated --- rapidtide/workflows/happy.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/rapidtide/workflows/happy.py b/rapidtide/workflows/happy.py index 5df9ff2e..5f441db5 100644 --- a/rapidtide/workflows/happy.py +++ b/rapidtide/workflows/happy.py @@ -77,28 +77,22 @@ def happy_main(argparsingfunc): # create the canary file Path(f"{outputroot}_ISRUNNING.txt").touch() - - # if we are running in a container, make sure we enforce memory limits properly - try: - testval = os.environ["RUNNING_IN_CONTAINER"] - except KeyError: - args.runningincontainer = False - else: - args.runningincontainer = True - args.containermemfree, args.containermemswap = tide_util.findavailablemem() - tide_util.setmemlimit(args.containermemfree) - # if running Apptainer/Singularity, this is necessary to enforce mmemory limits properly + # if running in Docker or Apptainer/Singularity, this is necessary to enforce mmemory limits properly # otherwise likely to error out in gzip.py or at voxelnormalize step try: - singularity_env = (os.environ.get("SINGULARITY_NAME") or os.environ.get("SINGULARITY_CONTAINER")) and not os.environ.get("CIRCLECI") + container_env = os.environ["RUNNING_IN_CONTAINER"] or os.environ.get("SINGULARITY_NAME") or os.environ.get("SINGULARITY_CONTAINER") except KeyError: args.runningincontainer = False else: args.runningincontainer = True - args.singularitymemfree, args.singularitymemswap = tide_util.findavailablemem() - tide_util.setmemlimit(args.singularitymemfree) - + args.containermemfree, args.containermemswap = tide_util.findavailablemem() + try: + cirlecienv = os.environ.get("CIRCLECI") + except KeyError: + tide_util.setmemlimit(args.containermemfree) + else: + print("running in CircleCI environment - not messing with memory") # Set up loggers for workflow setup_logger( From 3582d0eecf13b717c0f4af8b0877d70c46f5f3d2 Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 11:34:33 -0500 Subject: [PATCH 04/10] Pulled container checking routine into util, shared with rapidtide --- rapidtide/util.py | 24 ++++++++++++++++++++++++ rapidtide/workflows/happy.py | 18 ++++++------------ rapidtide/workflows/rapidtide.py | 17 ++++++++++------- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/rapidtide/util.py b/rapidtide/util.py index 5d1a1e4f..e6f6c51a 100644 --- a/rapidtide/util.py +++ b/rapidtide/util.py @@ -132,6 +132,30 @@ def findavailablemem(): return free, swap +def checkifincontainer(): + # if running in Docker or Apptainer/Singularity, this is necessary to enforce mmemory limits properly + # otherwise likely to error out in gzip.py or at voxelnormalize step + try: + dummy = os.environ.get("RUNNING_IN_CONTAINER") + except KeyError: + try: + dummy = os.environ.get("SINGULARITY_CONTAINER") + except KeyError: + containertype = None + else: + containertype = "Singularity" + else: + containertype = "Docker" + + try: + dummy = os.environ.get("CIRCLECI") + except KeyError: + pass + else: + containertype = "CircleCI" + return containertype + + def setmemlimit(memlimit): resource.setrlimit(resource.RLIMIT_AS, (memlimit, memlimit)) diff --git a/rapidtide/workflows/happy.py b/rapidtide/workflows/happy.py index 5f441db5..d518f833 100644 --- a/rapidtide/workflows/happy.py +++ b/rapidtide/workflows/happy.py @@ -77,19 +77,13 @@ def happy_main(argparsingfunc): # create the canary file Path(f"{outputroot}_ISRUNNING.txt").touch() - + # if running in Docker or Apptainer/Singularity, this is necessary to enforce mmemory limits properly - # otherwise likely to error out in gzip.py or at voxelnormalize step - try: - container_env = os.environ["RUNNING_IN_CONTAINER"] or os.environ.get("SINGULARITY_NAME") or os.environ.get("SINGULARITY_CONTAINER") - except KeyError: - args.runningincontainer = False - else: - args.runningincontainer = True - args.containermemfree, args.containermemswap = tide_util.findavailablemem() - try: - cirlecienv = os.environ.get("CIRCLECI") - except KeyError: + # otherwise likely to error out in gzip.py or at voxelnormalize step. But do nothing if running in CircleCI + args.containertype = tide_util.checkifincontainer() + if args.containertype is not None: + if args.containertype != "CircleCI": + args.containermemfree, args.containermemswap = tide_util.findavailablemem() tide_util.setmemlimit(args.containermemfree) else: print("running in CircleCI environment - not messing with memory") diff --git a/rapidtide/workflows/rapidtide.py b/rapidtide/workflows/rapidtide.py index 76c2934c..5eeab716 100755 --- a/rapidtide/workflows/rapidtide.py +++ b/rapidtide/workflows/rapidtide.py @@ -269,14 +269,17 @@ def rapidtide_main(argparsingfunc): gc.enable() print("turning on garbage collection") - # if we are running in a container, make sure we enforce memory limits properly - if "RUNNING_IN_CONTAINER" in os.environ: - optiondict["runningincontainer"] = True - optiondict["containermemfree"], optiondict["containermemswap"] = tide_util.findavailablemem() - if optiondict["containermemfix"]: + # if running in Docker or Apptainer/Singularity, this is necessary to enforce mmemory limits properly + # otherwise likely to error out in gzip.py or at voxelnormalize step. But do nothing if running in CircleCI + optiondict["containertype"] = tide_util.checkifincontainer() + if optiondict["containertype"] is not None: + if optiondict["containertype"] != "CircleCI": + optiondict["containermemfree"], optiondict["containermemswap"] = ( + tide_util.findavailablemem() + ) tide_util.setmemlimit(optiondict["containermemfree"]) - else: - optiondict["runningincontainer"] = False + else: + print("running in CircleCI environment - not messing with memory") # write out the current version of the run options optiondict["currentstage"] = "init" From e5ed28b15411f1fbc92a4471344a627d57b4cf08 Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 11:42:10 -0500 Subject: [PATCH 05/10] Update CHANGELOG.md --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba6b3059..5df78c1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # Release history -## Version 3.0alpha2 (12/20/24) -* (package) Now includes codespell checking thank you to Yaroslav O. Halchenko (yarikoptic)! +## Version 3.0alpha2 (12/30/24) +* (happy, rapidtide) Now do more sophisticated container checkin to handle both Docker and Singularity. Thank you to Derek Monroe (https://github.com/dcmonroe) for the catch and the fix! +* (package) Now includes codespell checking thanks to Yaroslav O. Halchenko (https://github.com/yarikoptic)! ## Version 3.0alpha1 (12/20/24) * (rapidtide) The ``--fixdelay`` option has been split into two options. ``--initialdelay DELAY`` lets you specify either a float that sets the starting delay for every voxel to that value, or a 3D file specifying the initial delay for each voxel. ``--nodelayfit`` determines whether the delay can be adjusted from its initial value. Closes https://github.com/bbfrederick/rapidtide/issues/171. KNOWN ISSUE: If you supply an initial delay map, instead of using the global mean, rapidtide should use the delays to make your first stage regressor. Currently that is not the case. From 41333a6d458400fbbdcc7fecb1b4241f5366faab Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 11:58:44 -0500 Subject: [PATCH 06/10] Updated to describe changes --- CHANGELOG.md | 2 +- pyproject.toml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5df78c1d..1b05c883 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release history ## Version 3.0alpha2 (12/30/24) -* (happy, rapidtide) Now do more sophisticated container checkin to handle both Docker and Singularity. Thank you to Derek Monroe (https://github.com/dcmonroe) for the catch and the fix! +* (happy, rapidtide) Now do more sophisticated container checking to handle both Docker and Singularity. Thank you to Derek Monroe (https://github.com/dcmonroe) for the catch and the fix! * (package) Now includes codespell checking thanks to Yaroslav O. Halchenko (https://github.com/yarikoptic)! ## Version 3.0alpha1 (12/20/24) diff --git a/pyproject.toml b/pyproject.toml index e2075df0..01996a84 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ authors = [ {name = "Daniel M. Drucker, Ph.D."}, {name = "Jeffrey N Stout"}, {name = "Yaroslav O. Halchenko"}, + {name = "Derek Monroe"}, ] [project.urls] From ee0b2f307834cc581ba7f8a3ea81cb400966b9b1 Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 12:04:01 -0500 Subject: [PATCH 07/10] Fixed some spelling errors --- rapidtide/util.py | 8 ++++++-- rapidtide/workflows/happy.py | 3 ++- rapidtide/workflows/rapidtide.py | 3 ++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/rapidtide/util.py b/rapidtide/util.py index e6f6c51a..da6ce613 100644 --- a/rapidtide/util.py +++ b/rapidtide/util.py @@ -133,8 +133,12 @@ def findavailablemem(): def checkifincontainer(): - # if running in Docker or Apptainer/Singularity, this is necessary to enforce mmemory limits properly - # otherwise likely to error out in gzip.py or at voxelnormalize step + # Determine if the program is running in a container. If so, we may need to adjust the python memory + # limits because they are not set properly. But check if we're running on CircleCI - it does not seem + # to like you twiddling with the container parameters. + # + # possible return values are: None, "Docker", "Singularity", and "CircleCI" + # try: dummy = os.environ.get("RUNNING_IN_CONTAINER") except KeyError: diff --git a/rapidtide/workflows/happy.py b/rapidtide/workflows/happy.py index d518f833..5b63f045 100644 --- a/rapidtide/workflows/happy.py +++ b/rapidtide/workflows/happy.py @@ -78,8 +78,9 @@ def happy_main(argparsingfunc): # create the canary file Path(f"{outputroot}_ISRUNNING.txt").touch() - # if running in Docker or Apptainer/Singularity, this is necessary to enforce mmemory limits properly + # if running in Docker or Apptainer/Singularity, this is necessary to enforce memory limits properly # otherwise likely to error out in gzip.py or at voxelnormalize step. But do nothing if running in CircleCI + # because it does NOT like you messing with the container. args.containertype = tide_util.checkifincontainer() if args.containertype is not None: if args.containertype != "CircleCI": diff --git a/rapidtide/workflows/rapidtide.py b/rapidtide/workflows/rapidtide.py index 5eeab716..cb4b6d9a 100755 --- a/rapidtide/workflows/rapidtide.py +++ b/rapidtide/workflows/rapidtide.py @@ -269,8 +269,9 @@ def rapidtide_main(argparsingfunc): gc.enable() print("turning on garbage collection") - # if running in Docker or Apptainer/Singularity, this is necessary to enforce mmemory limits properly + # if running in Docker or Apptainer/Singularity, this is necessary to enforce memory limits properly # otherwise likely to error out in gzip.py or at voxelnormalize step. But do nothing if running in CircleCI + # because it does NOT like you messing with the container. optiondict["containertype"] = tide_util.checkifincontainer() if optiondict["containertype"] is not None: if optiondict["containertype"] != "CircleCI": From 3705fa502f1c0e4ad6bfd17f6843ae99fe0524eb Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 12:43:40 -0500 Subject: [PATCH 08/10] Fixed a logic error --- rapidtide/util.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/rapidtide/util.py b/rapidtide/util.py index da6ce613..9f80097c 100644 --- a/rapidtide/util.py +++ b/rapidtide/util.py @@ -151,12 +151,13 @@ def checkifincontainer(): else: containertype = "Docker" - try: - dummy = os.environ.get("CIRCLECI") - except KeyError: - pass - else: - containertype = "CircleCI" + if containertype is not None: + try: + dummy = os.environ.get("CIRCLECI") + except KeyError: + pass + else: + containertype = "CircleCI" return containertype From 2bcd9436b5d031d5169f74a3868e3157d437a48b Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 13:16:43 -0500 Subject: [PATCH 09/10] Fixed a GCE about how os.eviron.get works --- rapidtide/util.py | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/rapidtide/util.py b/rapidtide/util.py index 9f80097c..42d0ed4e 100644 --- a/rapidtide/util.py +++ b/rapidtide/util.py @@ -139,25 +139,14 @@ def checkifincontainer(): # # possible return values are: None, "Docker", "Singularity", and "CircleCI" # - try: - dummy = os.environ.get("RUNNING_IN_CONTAINER") - except KeyError: - try: - dummy = os.environ.get("SINGULARITY_CONTAINER") - except KeyError: - containertype = None - else: - containertype = "Singularity" - else: + if os.environ.get("RUNNING_IN_CONTAINER") is not None: containertype = "Docker" - - if containertype is not None: - try: - dummy = os.environ.get("CIRCLECI") - except KeyError: - pass - else: - containertype = "CircleCI" + elif os.environ.get("SINGULARITY_CONTAINER") is not None: + containertype = "Singularity" + else: + containertype = None + if os.environ.get("CIRCLECI") is not None: + containertype = "CircleCI" return containertype From a2d098d706d6bcf563d4f6873ad9463fccd27d21 Mon Sep 17 00:00:00 2001 From: Blaise deB Frederick Date: Mon, 30 Dec 2024 13:27:31 -0500 Subject: [PATCH 10/10] Test for singularity first --- rapidtide/util.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rapidtide/util.py b/rapidtide/util.py index 42d0ed4e..931719ef 100644 --- a/rapidtide/util.py +++ b/rapidtide/util.py @@ -139,10 +139,10 @@ def checkifincontainer(): # # possible return values are: None, "Docker", "Singularity", and "CircleCI" # - if os.environ.get("RUNNING_IN_CONTAINER") is not None: - containertype = "Docker" - elif os.environ.get("SINGULARITY_CONTAINER") is not None: + if os.environ.get("SINGULARITY_CONTAINER") is not None: containertype = "Singularity" + elif os.environ.get("RUNNING_IN_CONTAINER") is not None: + containertype = "Docker" else: containertype = None if os.environ.get("CIRCLECI") is not None: