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.

Sign up for GitHub

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

Add fix and test for metadata enable false bug #427

Merged
merged 2 commits into from
Apr 4, 2024
Merged

Add fix and test for metadata enable false bug #427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4ff7d4
Add fix and test for metadata enable false bug #426
jo-basevi Mar 15, 2024
b863724
Change metadata enable false option logic to pre-branching-uuid-changes
jo-basevi Apr 2, 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
4 changes: 2 additions & 2 deletions docs/source/config.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,8 @@ Experiment Tracking

``enable`` (*Default:* ``True``)
Flag to enable/disable creating/updating metadata files and UUIDs.
If set to False, the UUID is left out of the experiment name used
for archival.
If set to False, the experiment name used for archival is either the
control directory name or the configured ``experiment`` name.

``model`` (*Default: The configured model value*)
Model name used when generating metadata for new experiments.
Expand Down
47 changes: 21 additions & 26 deletions payu/metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,12 @@ def __init__(self,
self.repo = GitRepository(self.control_path, catch_error=True)

self.branch = branch
self.branch_uuid_experiment = True

# Set uuid if in metadata file
metadata = self.read_file()
self.uuid = metadata.get(UUID_FIELD, None)
self.uuid_updated = False

# Experiment name configuration - this overrides experiment name
self.config_experiment_name = self.config.get("experiment", None)

def read_file(self) -> CommentedMap:
"""Read metadata file - preserving orginal format if it exists"""
metadata = CommentedMap()
Expand All @@ -118,8 +114,8 @@ def setup(self,
directories in the Laboratory.
"""
if not self.enabled:
# Set experiment name only - either configured or includes branch
self.set_experiment_name(ignore_uuid=True)
# Set experiment name only - either configured or legacy name
self.set_experiment_name()

elif self.uuid is not None and (keep_uuid or not is_new_experiment):
self.set_experiment_name(keep_uuid=keep_uuid,
Expand All @@ -133,7 +129,7 @@ def setup(self,

self.archive_path = self.lab_archive_path / self.experiment_name

def new_experiment_name(self, ignore_uuid: bool = False) -> str:
def new_experiment_name(self) -> str:
"""Generate a new experiment name"""
if self.branch is None:
self.branch = self.repo.get_branch_name()
Expand All @@ -142,39 +138,39 @@ def new_experiment_name(self, ignore_uuid: bool = False) -> str:
adding_branch = self.branch not in (None, 'main', 'master')
suffix = f'-{self.branch}' if adding_branch else ''

if not ignore_uuid:
truncated_uuid = self.uuid[:TRUNCATED_UUID_LENGTH]
suffix += f'-{truncated_uuid}'
truncated_uuid = self.uuid[:TRUNCATED_UUID_LENGTH]
suffix += f'-{truncated_uuid}'

return self.control_path.name + suffix

def set_experiment_name(self,
is_new_experiment: bool = False,
keep_uuid: bool = False,
ignore_uuid: bool = False) -> None:
keep_uuid: bool = False) -> None:
"""Set experiment name - this is used for work and archive
sub-directories in the Laboratory"""
if self.config_experiment_name is not None:
# The configured value over-rides the experiment name
self.experiment_name = self.config_experiment_name
self.branch_uuid_experiment = False
# Experiment name configuration - this overrides experiment name
self.experiment_name = self.config.get("experiment", None)
if self.experiment_name is not None:
print(f"Experiment name is configured in config.yaml: ",
self.experiment_name)
return

if ignore_uuid:
# Leave experiment UUID out of experiment name
self.experiment_name = self.new_experiment_name(ignore_uuid=True)
# Legacy experiment name and archive path
legacy_name = self.control_path.name
legacy_archive_path = self.lab_archive_path / legacy_name

if not self.enabled:
# Metadata/UUID generation is disabled, so leave UUID out of
# experiment name
self.experiment_name = legacy_name
print("Metadata is disabled in config.yaml.",
f"Experiment name used for archival: {self.experiment_name}")
return

# Branch-UUID experiment name and archive path
branch_uuid_experiment_name = self.new_experiment_name()
archive_path = self.lab_archive_path / branch_uuid_experiment_name

# Legacy experiment name and archive path
legacy_name = self.control_path.name
legacy_archive_path = self.lab_archive_path / legacy_name

if is_new_experiment or archive_path.exists():
# Use branch-UUID aware experiment name
self.experiment_name = branch_uuid_experiment_name
Expand All @@ -183,7 +179,6 @@ def set_experiment_name(self,
self.experiment_name = legacy_name
print(f"Pre-existing archive found at: {legacy_archive_path}. "
f"Experiment name will remain: {legacy_name}")
self.branch_uuid_experiment = False
elif keep_uuid:
# Use same experiment UUID and use branch-UUID name for archive
self.experiment_name = branch_uuid_experiment_name
Expand All @@ -202,7 +197,7 @@ def set_new_uuid(self, is_new_experiment: bool = False) -> None:
self.set_experiment_name(is_new_experiment=is_new_experiment)

# If experiment name does not include UUID, leave it unchanged
if not self.branch_uuid_experiment:
if self.experiment_name.endswith(self.uuid[:TRUNCATED_UUID_LENGTH]):
return

# Check experiment name is unique in local archive
Expand Down Expand Up @@ -267,7 +262,7 @@ def update_file(self,

# Add extra fields if new branch-uuid experiment
# so to not over-write fields if it's a pre-existing legacy experiment
if self.branch_uuid_experiment:
if self.experiment_name.endswith(self.uuid[:TRUNCATED_UUID_LENGTH]):
metadata[CREATED_FIELD] = datetime.now().strftime('%Y-%m-%d')
metadata[NAME_FIELD] = self.experiment_name
metadata[MODEL_FIELD] = self.get_model_name()
Expand Down
46 changes: 24 additions & 22 deletions test/test_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,17 @@ def setup_and_teardown():

@patch("payu.metadata.GitRepository")
@pytest.mark.parametrize(
"uuid, legacy_archive_exists, previous_metadata, expected_metadata",
"uuid, experiment_name, previous_metadata, expected_metadata",
[
# Test new metadata file created
(
"b1f3ce3d-99da-40e4-849a-c8b352948a31",
False,
"ctrl-branch-b1f3ce3d",
None,
{
"experiment_uuid": "b1f3ce3d-99da-40e4-849a-c8b352948a31",
"created": '2000-01-01',
"name": "DefaultExperimentName",
"name": "ctrl-branch-b1f3ce3d",
"model": "TEST-MODEL",
"url": "mockUrl",
"contact": "mockUser",
Expand All @@ -99,7 +99,7 @@ def setup_and_teardown():
# Test metadata file updated when new UUID
(
"7b90f37c-4619-44f9-a439-f76fdf6ae2bd",
False,
"Control-Branch-7b90f37c",
{
"experiment_uuid": "b3298c7f-01f6-4f0a-be32-ce5d2cfb9a04",
"contact": "Add your name here",
Expand All @@ -110,7 +110,7 @@ def setup_and_teardown():
"experiment_uuid": "7b90f37c-4619-44f9-a439-f76fdf6ae2bd",
"description": "Add description here",
"created": '2000-01-01',
"name": "DefaultExperimentName",
"name": "Control-Branch-7b90f37c",
"model": "TEST-MODEL",
"url": "mockUrl",
"contact": "mockUser",
Expand All @@ -120,7 +120,7 @@ def setup_and_teardown():
# Test extra fields not added with legacy experiments
(
"7b90f37c-4619-44f9-a439-f76fdf6ae2bd",
True,
"ctrl",
{
"experiment_uuid": "0f49f2a0-f45e-4c0b-a3b6-4b0bf21f2b75",
"name": "UserDefinedExperimentName",
Expand All @@ -136,7 +136,7 @@ def setup_and_teardown():
),
]
)
def test_update_file(mock_repo, uuid, legacy_archive_exists,
def test_update_file(mock_repo, uuid, experiment_name,
previous_metadata, expected_metadata):
# Create pre-existing metadata file
metadata_path = ctrldir / 'metadata.yaml'
Expand All @@ -158,8 +158,7 @@ def test_update_file(mock_repo, uuid, legacy_archive_exists,
with cd(ctrldir):
metadata = Metadata(archive_dir)
metadata.uuid = uuid
metadata.experiment_name = "DefaultExperimentName"
metadata.branch_uuid_experiment = not legacy_archive_exists
metadata.experiment_name = experiment_name

# Mock datetime (for created date)
with patch('payu.metadata.datetime') as mock_date:
Expand Down Expand Up @@ -300,22 +299,25 @@ def test_new_experiment_name(branch, expected_name):
assert experiment == expected_name


@pytest.mark.parametrize(
"branch, expected_name",
[(None, "ctrl"),
("main", "ctrl"),
("branch", "ctrl-branch")]
)
def test_new_experiment_name_ignore_uuid(branch, expected_name):
# Test configured experiment name is the set experiment name
with cd(ctrldir):
metadata = Metadata(archive_dir)
def test_metadata_enable_false():
# Set metadata to false in config file
test_config = copy.deepcopy(config)
test_config['metadata'] = {
"enable": False
}
write_config(test_config)

with patch('payu.metadata.GitRepository.get_branch_name') as mock_branch:
mock_branch.return_value = branch
experiment = metadata.new_experiment_name(ignore_uuid=True)
mock_branch.return_value = "mock-branch"

assert experiment == expected_name
with cd(ctrldir):
metadata = Metadata(archive_dir)
metadata.setup()
metadata.write_metadata()

# Test UUID kept out of experiment name and metadata file is not written
assert metadata.experiment_name == "ctrl"
assert not (ctrldir / "metadata.yaml").exists()


@patch("payu.metadata.GitRepository")
Expand Down
Loading