Fix Entities Default Path

.pylintrc

@@ -32,10 +32,6 @@ jobs=4
 # Pickle collected data for later comparisons.
 persistent=yes
 
-# When enabled, pylint would attempt to guess common misconfiguration and emit
-# user-friendly hints instead of false-positive error messages.
-suggestion-mode=yes
-
 # Ignore problematic extensions
 extension-pkg-whitelist=pydantic,
 

doc/changelog.md

@@ -26,6 +26,8 @@ Description
 - Add instructions for installing SmartSim on PML's Scylla
 - Drop unsued development dependencies
 - Fix typos in documentation
+- Changed default path when manually constructing `Model`s, `Ensemble`s and
+  `Orchestrator`s to the current working directory
 
 Detailed Notes
 
@@ -108,6 +110,14 @@ Detailed Notes
 - Removes an Numpy upper bound in the SmartSim dependency list now that
   SmartRedis and supported ML backends support Numpy 2.0.
   ([SmartSim-PR803](https://github.com/CrayLabs/SmartSim/pull/803))
+- When constructing a `Model`, `Ensemble`, or `Orchestrator`, the `__init__`
+  method was type hinted as able to accept a `None` value. This was supposed to
+  act as a sentinel that would default to the current working directory. There
+  was a bug in how this resolved such that if a `path` argument was not
+  provided it would resolve to the working directory at import time and if
+  `path` was explicitly passed in as `None` it would resolve to the string
+  `"None"` this has since been corrected to the expected behavior.
+  ([SmartSim-PR808](https://github.com/CrayLabs/SmartSim/pull/808))
 
 
 ### 0.8.0

smartsim/database/orchestrator.py

@@ -163,7 +163,7 @@ class Orchestrator(EntityList[DBNode]):
 
     def __init__(
         self,
-        path: str | None = getcwd(),
+        path: str | None = None,
         port: int = 6379,
         interface: str | list[str] = "lo",
         launcher: str = "local",
@@ -229,7 +229,7 @@ def __init__(
 
         super().__init__(
             name=db_identifier,
-            path=str(path),
+            path=path if path is not None else getcwd(),
             port=port,
             interface=interface,
             db_nodes=db_nodes,

smartsim/entity/ensemble.py

@@ -62,7 +62,7 @@ def __init__(
         self,
         name: str,
         params: dict[str, t.Any],
-        path: str | None = getcwd(),
+        path: str | None = None,
         params_as_args: list[str] | None = None,
         batch_settings: BatchSettings | None = None,
         run_settings: RunSettings | None = None,
@@ -96,7 +96,12 @@ def __init__(
         self.run_settings = run_settings
         self.replicas: str
 
-        super().__init__(name, str(path), perm_strat=perm_strat, **kwargs)
+        super().__init__(
+            name,
+            path if path is not None else getcwd(),
+            perm_strat=perm_strat,
+            **kwargs,
+        )
 
     @property
     def models(self) -> Collection[Model]:

smartsim/entity/entityList.py

@@ -34,7 +34,7 @@
     import smartsim
 
 _T = t.TypeVar("_T", bound=SmartSimEntity)
-# Old style pyint from TF 2.6.x does not know about pep484 style ``TypeVar`` names
+# Old style pylint from TF 2.6.x does not know about pep484 style ``TypeVar`` names
 # pylint: disable-next=invalid-name
 _T_co = t.TypeVar("_T_co", bound=SmartSimEntity, covariant=True)
 

smartsim/entity/model.py

@@ -74,7 +74,7 @@ def __init__(
         name: str,
         params: dict[str, str],
         run_settings: RunSettings,
-        path: str | None = getcwd(),
+        path: str | None = None,
         params_as_args: list[str] | None = None,
         batch_settings: BatchSettings | None = None,
     ):
@@ -91,7 +91,7 @@ def __init__(
         :param batch_settings: Launcher settings for running the individual
                                model as a batch job
         """
-        super().__init__(name, str(path), run_settings)
+        super().__init__(name, path if path is not None else getcwd(), run_settings)
         self.params = _parse_model_parameters(params)
         self.params_as_args = params_as_args
         self.incoming_entities: list[SmartSimEntity] = []

tests/test_ensemble.py

@@ -25,6 +25,8 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
+import os
+import os.path
 from copy import deepcopy
 
 import pytest
@@ -303,3 +305,25 @@ def test_ensemble_type(test_dir):
     ens_settings = RunSettings("python")
     ensemble = exp.create_ensemble("name", replicas=4, run_settings=ens_settings)
     assert ensemble.type == "Ensemble"
+
+
+@pytest.mark.parametrize(
+    "kwargs",
+    [
+        pytest.param({}, id="Default Argument"),
+        pytest.param({"path": None}, id="Explicit `path=None`"),
+    ],
+)
+def test_ensemble_path_defaults_to_cwd(monkeypatch, test_dir, kwargs):
+    rs = RunSettings("echo", exe_args=["spam", "eggs"])
+    test_subdir = os.path.join(test_dir, "subdir")
+    os.mkdir(test_subdir)
+    with monkeypatch.context() as ctx:
+        ctx.chdir(test_dir)
+        e1 = Ensemble("root-ens", {}, run_settings=rs, replicas=3, **kwargs)
+    with monkeypatch.context() as ctx:
+        ctx.chdir(test_subdir)
+        e2 = Ensemble("subdir-ens", {}, run_settings=rs, replicas=3, **kwargs)
+    assert e1.path == test_dir
+    assert e2.path == test_subdir
+    assert all(os.path.join(e.path, m.name) == m.path for e in (e1, e2) for m in e)

tests/test_model.py

@@ -24,6 +24,8 @@
 # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import os
+import os.path
 from uuid import uuid4
 
 import numpy as np
@@ -196,3 +198,24 @@ def test_good_model_params(dtype):
 def test_bad_model_params(bad_val):
     with pytest.raises(TypeError):
         _parse_model_parameters({"foo": bad_val})
+
+
+@pytest.mark.parametrize(
+    "kwargs",
+    [
+        pytest.param({}, id="Default Argument"),
+        pytest.param({"path": None}, id="Explicit `path=None`"),
+    ],
+)
+def test_model_path_defaults_to_cwd(monkeypatch, test_dir, kwargs):
+    rs = RunSettings("echo", exe_args=["spam", "eggs"])
+    test_subdir = os.path.join(test_dir, "subdir")
+    os.mkdir(test_subdir)
+    with monkeypatch.context() as ctx:
+        ctx.chdir(test_dir)
+        m1 = Model("root-model", {}, run_settings=rs, **kwargs)
+    with monkeypatch.context() as ctx:
+        ctx.chdir(test_subdir)
+        m2 = Model("subdir-model", {}, run_settings=rs, **kwargs)
+    assert m1.path == test_dir
+    assert m2.path == test_subdir

tests/test_orchestrator.py

@@ -25,6 +25,8 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
+import os
+import os.path
 import typing as t
 
 import psutil
@@ -119,6 +121,27 @@ def test_multiple_interfaces(
     exp.stop(db)
 
 
+@pytest.mark.parametrize(
+    "kwargs",
+    [
+        pytest.param({}, id="Default Argument"),
+        pytest.param({"path": None}, id="Explicit `path=None`"),
+    ],
+)
+def test_orchestrator_path_defaults_to_cwd(monkeypatch, test_dir, kwargs):
+    test_subdir = os.path.join(test_dir, "subdir")
+    os.mkdir(test_subdir)
+    with monkeypatch.context() as ctx:
+        ctx.chdir(test_dir)
+        db1 = Orchestrator(launcher="local", **kwargs)
+    with monkeypatch.context() as ctx:
+        ctx.chdir(test_subdir)
+        db2 = Orchestrator(launcher="local", **kwargs)
+    assert db1.path == test_dir
+    assert db2.path == test_subdir
+    assert all(db.path == n.path for db in (db1, db2) for n in db)
+
+
 def test_catch_local_db_errors() -> None:
     # local database with more than one node not allowed
     with pytest.raises(SSUnsupportedError):