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

Ensemble.files refactor #746

Open
wants to merge 45 commits into
base: smartsim-refactor
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
3cac1d1
initial push
amandarichardsonn Sep 30, 2024
b6b5022
operations module close to finish
amandarichardsonn Sep 30, 2024
6e88a40
styling and mypy and partial test passing
amandarichardsonn Sep 30, 2024
c93c85d
saving spot
amandarichardsonn Oct 1, 2024
3866bbd
very ugly testing
amandarichardsonn Oct 1, 2024
b2c5421
adding configuration tests
amandarichardsonn Oct 3, 2024
e221b43
pushing updates
amandarichardsonn Oct 3, 2024
69f7866
remove unused print statements
amandarichardsonn Oct 3, 2024
143ff4b
fix typos
amandarichardsonn Oct 3, 2024
cdd82a4
updates to PR
amandarichardsonn Oct 7, 2024
56eb31c
initial push
amandarichardsonn Oct 8, 2024
83e015a
partial address matts comments
amandarichardsonn Oct 8, 2024
70dd2a0
addressed Matts comments, exclude Qs
amandarichardsonn Oct 9, 2024
b9c0137
update to ensemble
amandarichardsonn Oct 9, 2024
44a3b18
Merge remote-tracking branch 'upstream/smartsim-refactor' into gen
amandarichardsonn Oct 9, 2024
3a2012e
updates
amandarichardsonn Oct 14, 2024
006a430
Merge remote-tracking branch 'origin/gen' into builder_op
amandarichardsonn Oct 14, 2024
6c2d908
updates
amandarichardsonn Oct 14, 2024
f87d7cd
styling up to date
amandarichardsonn Oct 15, 2024
5de9628
Merge remote-tracking branch 'origin/gen' into builder_op
amandarichardsonn Oct 15, 2024
a7960f1
file rename
amandarichardsonn Oct 15, 2024
a6391cf
remove unused fixtures from operations
amandarichardsonn Oct 15, 2024
96c21bd
Merge remote-tracking branch 'origin/gen' into builder_op
amandarichardsonn Oct 15, 2024
e937f0d
small updates but pushing bc issue in gen branch
amandarichardsonn Oct 15, 2024
36e8801
adding docstrings
amandarichardsonn Oct 15, 2024
1fe7719
pushing for an idea in gen
amandarichardsonn Oct 15, 2024
b1dbbb3
adding default tag
amandarichardsonn Oct 15, 2024
f42e208
Merge remote-tracking branch 'origin/gen' into builder_op
amandarichardsonn Oct 15, 2024
8c7a767
testing operations complete
amandarichardsonn Oct 15, 2024
06f530d
ensemble op test complete
amandarichardsonn Oct 15, 2024
8a9a577
pushing to switch branches
amandarichardsonn Oct 15, 2024
e88f226
pushing test updates
amandarichardsonn Oct 15, 2024
d60df2d
Merge remote-tracking branch 'origin/gen' into builder_op
amandarichardsonn Oct 15, 2024
d632588
pushing to PR
amandarichardsonn Oct 15, 2024
27aa710
remove unused EntityFile imports
amandarichardsonn Oct 15, 2024
2f9838f
Merge remote-tracking branch 'origin/gen' into builder_op
amandarichardsonn Oct 15, 2024
7bc70f2
remove EntityFiles
amandarichardsonn Oct 15, 2024
b71871e
remove dead fixture
amandarichardsonn Oct 15, 2024
87372e1
changing types fro t.List to list
amandarichardsonn Oct 15, 2024
a8965bd
Merge remote-tracking branch 'origin/gen' into builder_op
amandarichardsonn Oct 15, 2024
bd2ac50
changing t.List to list
amandarichardsonn Oct 15, 2024
6198a14
address one comment from Matt
amandarichardsonn Oct 16, 2024
8ca13c3
Merge remote-tracking branch 'origin/gen' into builder_op
amandarichardsonn Oct 16, 2024
c039680
failing
amandarichardsonn Oct 16, 2024
f4e4cdb
Merge branch 'smartsim-refactor' into builder_op
amandarichardsonn Oct 16, 2024
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
165 changes: 165 additions & 0 deletions smartsim/_core/generation/operations/ensemble_operations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
import pathlib
import typing as t
from dataclasses import dataclass, field

from .operations import default_tag
from .utils.helpers import check_src_and_dest_path


class EnsembleGenerationProtocol(t.Protocol):
"""Protocol for Ensemble Generation Operations."""

src: pathlib.Path
"""Path to source"""
dest: t.Optional[pathlib.Path]
"""Path to destination"""


class EnsembleCopyOperation(EnsembleGenerationProtocol):
"""Ensemble Copy Operation"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider describing the functionality.

"""
A file generation operation used to specify parameters
for copying a file into a location accessible to the Ensemble.
"""

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better description than my three words eep! will change


def __init__(
self, src: pathlib.Path, dest: t.Optional[pathlib.Path] = None
) -> None:
"""Initialize a EnsembleCopyOperation object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the result if the destination path is not supplied. Should this behavior be specified in this docstring?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I will specify


:param src: Path to source
:param dest: Path to destination
"""
check_src_and_dest_path(src, dest)
self.src = src
"""Path to source"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider being explicit Path to the file that will be copied

self.dest = dest
"""Path to destination"""


class EnsembleSymlinkOperation(EnsembleGenerationProtocol):
"""Ensemble Symlink Operation"""

def __init__(
self, src: pathlib.Path, dest: t.Optional[pathlib.Path] = None
) -> None:
"""Initialize a EnsembleSymlinkOperation object

:param src: Path to source
:param dest: Path to destination
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the result if the destination path is not supplied. Should this behavior be specified in this docstring?

"""
check_src_and_dest_path(src, dest)
self.src = src
"""Path to source"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider being explicit: Path to the file that will be symlink'ed

self.dest = dest
"""Path to destination"""


class EnsembleConfigureOperation(EnsembleGenerationProtocol):
"""Ensemble Configure Operation"""

def __init__(
self,
src: pathlib.Path,
file_parameters: t.Mapping[str, t.Sequence[str]],
dest: t.Optional[pathlib.Path] = None,
tag: t.Optional[str] = None,
) -> None:
"""Initialize a EnsembleConfigureOperation

:param src: Path to source
:param file_parameters: File parameters to find and replace
:param dest: Path to destination
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the result if the destination path is not supplied. Should this behavior be specified in this docstring?

:param tag: Tag to use for find and replacement
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the result if the tag is not supplied. Should this behavior be specified in this docstring?

"""
check_src_and_dest_path(src, dest)
self.src = src
"""Path to source"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider being explicit: Path to the template file that will be modified

self.dest = dest
"""Path to destination"""
self.file_parameters = file_parameters
"""File parameters to find and replace"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider being explicit in what file parameters are.

A mapping that contains the string to replace in the source file as the key with the replacement string as the corresponding value.

self.tag = tag if tag else default_tag
"""Tag to use for the file"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is a tag?



EnsembleGenerationProtocolT = t.TypeVar(
"EnsembleGenerationProtocolT", bound=EnsembleGenerationProtocol
)


@dataclass
class EnsembleFileSysOperationSet:
"""Dataclass to represent a set of Ensemble file system operation objects"""
Copy link
Contributor

@ankona ankona Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omit dataclass

Suggested change
"""Dataclass to represent a set of Ensemble file system operation objects"""
"""A collection of Ensemble file system operation objects to apply during file generation."""


operations: list[EnsembleGenerationProtocol] = field(default_factory=list)
"""Set of Ensemble file system objects that match the EnsembleGenerationProtocol"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

says set but it is a list. might be misleading. consider using list/collection instead


def add_copy(
self, src: pathlib.Path, dest: t.Optional[pathlib.Path] = None
) -> None:
"""Add a copy operation to the operations list

:param src: Path to source
:param dest: Path to destination
"""
self.operations.append(EnsembleCopyOperation(src, dest))

def add_symlink(
self, src: pathlib.Path, dest: t.Optional[pathlib.Path] = None
) -> None:
"""Add a symlink operation to the operations list

:param src: Path to source
:param dest: Path to destination
"""
self.operations.append(EnsembleSymlinkOperation(src, dest))

def add_configuration(
self,
src: pathlib.Path,
file_parameters: t.Mapping[str, t.Sequence[str]],
dest: t.Optional[pathlib.Path] = None,
tag: t.Optional[str] = None,
) -> None:
"""Add a configure operation to the operations list

:param src: Path to source
:param file_parameters: File parameters to find and replace
:param dest: Path to destination
:param tag: Tag to use for find and replacement
"""
self.operations.append(
EnsembleConfigureOperation(src, file_parameters, dest, tag)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we change the order of these so that it matches the configure method?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I dont know if file_parameters is the best name for what is going in? This is a mapping of the items that need to be found and replaced right?

)

@property
def copy_operations(self) -> list[EnsembleCopyOperation]:
"""Property to get the list of copy files.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. omit property.
  2. replace files with operations to be explicit that the files specified therein are not returned
Suggested change
"""Property to get the list of copy files.
"""Retrieve the list of copy operations.


:return: List of EnsembleCopyOperation objects
"""
return self._filter(EnsembleCopyOperation)

@property
def symlink_operations(self) -> list[EnsembleSymlinkOperation]:
"""Property to get the list of symlink files.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. omit property.
  2. replace files with operations to be explicit that the files specified therein are not returned
Suggested change
"""Property to get the list of symlink files.
"""Retrieve the list of symlink operations.


:return: List of EnsembleSymlinkOperation objects
"""
return self._filter(EnsembleSymlinkOperation)

@property
def configure_operations(self) -> list[EnsembleConfigureOperation]:
"""Property to get the list of configure files.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. omit property.
  2. replace files with operations to be explicit that the files specified therein are not returned
Suggested change
"""Property to get the list of configure files.
"""Retrieve the list of configure operations.


:return: List of EnsembleConfigureOperation objects
"""
return self._filter(EnsembleConfigureOperation)

def _filter(
self, type: t.Type[EnsembleGenerationProtocolT]
) -> list[EnsembleGenerationProtocolT]:
"""Filters the operations list to include only instances of the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Filters the operations list to include only instances of the
"""Filters the operations list by operation type

specified type.

:param type: The type of operations to filter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:param type: The type of operations to filter
:param type: The type of operations to include in results

:return: A list of operations that are instances of the specified type
"""
return [x for x in self.operations if isinstance(x, type)]
Loading
Loading