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

Mitigate suppressed protected-access errors from pylint #341

Merged
merged 7 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions smartsim/_core/control/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,28 +636,26 @@ def _set_dbobjects(self, manifest: Manifest) -> None:

for model in manifest.models:
if not model.colocated:
# pylint: disable=protected-access
for db_model in model._db_models:
for db_model in model.db_models:
set_ml_model(db_model, client)
for db_script in model._db_scripts:
for db_script in model.db_scripts:
set_script(db_script, client)

for ensemble in manifest.ensembles:
# pylint: disable=protected-access
for db_model in ensemble._db_models:
for db_model in ensemble.db_models:
set_ml_model(db_model, client)
for db_script in ensemble._db_scripts:
for db_script in ensemble.db_scripts:
set_script(db_script, client)
for entity in ensemble.models:
if not entity.colocated:
# Set models which could belong only
# to the entities and not to the ensemble
# but avoid duplicates
for db_model in entity._db_models:
if db_model not in ensemble._db_models:
for db_model in entity.db_models:
if db_model not in ensemble.db_models:
set_ml_model(db_model, client)
for db_script in entity._db_scripts:
if db_script not in ensemble._db_scripts:
for db_script in entity.db_scripts:
if db_script not in ensemble.db_scripts:
set_script(db_script, client)


Expand Down
6 changes: 2 additions & 4 deletions smartsim/_core/control/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,10 @@ def has_db_objects(self) -> bool:
"""Check if any entity has DBObjects to set"""

def has_db_models(entity: t.Union[EntityList, Model]) -> bool:
# pylint: disable=protected-access
return len(entity._db_models) > 0
return len(list(entity.db_models)) > 0

def has_db_scripts(entity: t.Union[EntityList, Model]) -> bool:
# pylint: disable=protected-access
return len(entity._db_scripts) > 0
return len(list(entity.db_scripts)) > 0

has_db_objects = False
for model in self.models:
Expand Down
2 changes: 1 addition & 1 deletion smartsim/_core/generation/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def _gen_entity_list_dir(self, entity_lists: t.List[Ensemble]) -> None:
mkdir(elist_dir)
elist.path = elist_dir

self._gen_entity_dirs(elist.models, entity_list=elist)
self._gen_entity_dirs(list(elist.models), entity_list=elist)

def _gen_entity_dirs(
self,
Expand Down
3 changes: 1 addition & 2 deletions smartsim/_core/launcher/step/cobaltStep.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ def _write_script(self) -> str:
for opt in self.batch_settings.format_batch_args():
script_file.write(f"#COBALT {opt}\n")

# pylint: disable-next=protected-access
for cmd in self.batch_settings._preamble:
for cmd in self.batch_settings.preamble:
script_file.write(f"{cmd}\n")

for i, cmd in enumerate(self.step_cmds):
Expand Down
3 changes: 1 addition & 2 deletions smartsim/_core/launcher/step/pbsStep.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ def _write_script(self) -> str:
for opt in self.batch_settings.format_batch_args():
script_file.write(f"#PBS {opt}\n")

# pylint: disable-next=protected-access
for cmd in self.batch_settings._preamble:
for cmd in self.batch_settings.preamble:
script_file.write(f"{cmd}\n")

for i, cmd in enumerate(self.step_cmds):
Expand Down
3 changes: 1 addition & 2 deletions smartsim/_core/launcher/step/slurmStep.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ def _write_script(self) -> str:
for opt in self.batch_settings.format_batch_args():
script_file.write(f"#SBATCH {opt}\n")

# pylint: disable-next=protected-access
for cmd in self.batch_settings._preamble:
for cmd in self.batch_settings.preamble:
script_file.write(f"{cmd}\n")

for i, cmd in enumerate(self.step_cmds):
Expand Down
15 changes: 7 additions & 8 deletions smartsim/entity/ensemble.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def __init__(
super().__init__(name, getcwd(), perm_strat=perm_strat, **kwargs)

@property
def models(self) -> t.List[Model]:
def models(self) -> t.Iterable[Model]:
"""
Helper property to cast self.entities to Model type for type correctness
"""
Expand Down Expand Up @@ -474,16 +474,15 @@ def add_function(

@staticmethod
def _extend_entity_db_models(model: Model, db_models: t.List[DBModel]) -> None:
# pylint: disable=protected-access
entity_db_models = [db_model.name for db_model in model._db_models]
entity_db_models = [db_model.name for db_model in model.db_models]

for db_model in db_models:
if not db_model.name in entity_db_models:
model._append_db_model(db_model)
if db_model.name not in entity_db_models:
model.add_ml_model_object(db_model)

@staticmethod
def _extend_entity_db_scripts(model: Model, db_scripts: t.List[DBScript]) -> None:
# pylint: disable=protected-access
entity_db_scripts = [db_script.name for db_script in model._db_scripts]
entity_db_scripts = [db_script.name for db_script in model.db_scripts]
for db_script in db_scripts:
if not db_script.name in entity_db_scripts:
model._append_db_script(db_script)
model.add_script_object(db_script)
13 changes: 12 additions & 1 deletion smartsim/entity/entityList.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
import typing as t

if t.TYPE_CHECKING:
import smartsim # pylint: disable=unused-import
# pylint: disable=unused-import
import smartsim

Check warning on line 31 in smartsim/entity/entityList.py

View check run for this annotation

Codecov / codecov/patch

smartsim/entity/entityList.py#L31

Added line #L31 was not covered by tests


class EntityList:
Expand All @@ -45,6 +46,16 @@
"""Initialize the SmartSimEntity objects in the container"""
raise NotImplementedError

@property
def db_models(self) -> t.Iterable["smartsim.entity.DBModel"]:
"""Return an immutable collection of attached models"""
return (model for model in self._db_models)

@property
def db_scripts(self) -> t.Iterable["smartsim.entity.DBScript"]:
"""Return an immutable collection of attached scripts"""
return (script for script in self._db_scripts)

@property
def batch(self) -> bool:
try:
Expand Down
21 changes: 16 additions & 5 deletions smartsim/entity/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
from ..settings.base import BatchSettings, RunSettings
from ..log import get_logger


logger = get_logger(__name__)

class Model(SmartSimEntity):
Expand Down Expand Up @@ -80,6 +81,16 @@ def __init__(
self._db_scripts: t.List[DBScript] = []
self.files: t.Optional[EntityFiles] = None

@property
def db_models(self) -> t.Iterable[DBModel]:
"""Return an immutable collection of attached models"""
return (model for model in self._db_models)

@property
def db_scripts(self) -> t.Iterable[DBScript]:
"""Return an immutable collection attached of scripts"""
return (script for script in self._db_scripts)

@property
def colocated(self) -> bool:
"""Return True if this Model will run with a colocated Orchestrator"""
Expand Down Expand Up @@ -461,7 +472,7 @@ def add_ml_model(
inputs=inputs,
outputs=outputs,
)
self._append_db_model(db_model)
self.add_ml_model_object(db_model)

def add_script(
self,
Expand Down Expand Up @@ -506,7 +517,7 @@ def add_script(
device=device,
devices_per_node=devices_per_node,
)
self._append_db_script(db_script)
self.add_script_object(db_script)

def add_function(
self,
Expand Down Expand Up @@ -543,7 +554,7 @@ def add_function(
db_script = DBScript(
name=name, script=function, device=device, devices_per_node=devices_per_node
)
self._append_db_script(db_script)
self.add_script_object(db_script)

def __hash__(self) -> int:
return hash(self.name)
Expand All @@ -566,7 +577,7 @@ def __str__(self) -> str: # pragma: no cover
entity_str += "DB Scripts: \n" + str(len(self._db_scripts)) + "\n"
return entity_str

def _append_db_model(self, db_model: DBModel) -> None:
def add_ml_model_object(self, db_model: DBModel) -> None:
if not db_model.is_file and self.colocated:
err_msg = "ML model can not be set from memory for colocated databases.\n"
err_msg += (
Expand All @@ -577,7 +588,7 @@ def _append_db_model(self, db_model: DBModel) -> None:

self._db_models.append(db_model)

def _append_db_script(self, db_script: DBScript) -> None:
def add_script_object(self, db_script: DBScript) -> None:
if db_script.func and self.colocated:
if not isinstance(db_script.func, str):
err_msg = (
Expand Down
5 changes: 5 additions & 0 deletions smartsim/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,11 @@ def add_preamble(self, lines: t.List[str]) -> None:
else:
raise TypeError("Expected str or List[str] for lines argument")

@property
def preamble(self) -> t.Iterable[str]:
"""Return an iterable of preamble clauses to be prepended to the batch file"""
return (clause for clause in self._preamble)

def __str__(self) -> str: # pragma: no-cover
string = f"Batch Command: {self._batch_cmd}"
if self.batch_args:
Expand Down
6 changes: 3 additions & 3 deletions tests/test_batch_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,10 @@ def test_preamble():
)

bsub.add_preamble(["single line"])
assert len(bsub._preamble) == 1
assert len(list(bsub.preamble)) == 1

bsub.add_preamble(["another line"])
assert len(bsub._preamble) == 2
assert len(list(bsub.preamble)) == 2

bsub.add_preamble(["first line", "last line"])
assert len(bsub._preamble) == 4
assert len(list(bsub.preamble)) == 4
2 changes: 1 addition & 1 deletion tests/test_lsf_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def test_bsub_batch_manual():
assert formatted == result
sbatch.add_preamble("module load gcc")
sbatch.add_preamble(["module load openmpi", "conda activate smartsim"])
assert sbatch._preamble == [
assert list(sbatch.preamble) == [
"module load gcc",
"module load openmpi",
"conda activate smartsim",
Expand Down
Loading