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

[Bug] summarize_results_all_tasks_all_devs only returns results for first seed found. #80

Open
eddiebergman opened this issue Apr 30, 2024 · 4 comments
Labels
bug Something isn't working
Milestone

Comments

@eddiebergman
Copy link
Contributor

Not really sure how the whole task and dev id's really work but from what I gather from this function the path goes something like this:

  • If sub_dir: workingdir/seed/sub_dir/task_id/dev_id.
  • Else: workingdir/seed/task_id/dev_id

However this function only takes it over the first valid seed dir found, with no knowledge of what this is.

def summarize_results_all_tasks_all_devs(
path, sub_dir="", file_name="summary", user_prior_dir=None
):
"""
Summarizes the results of all tasks and all development stages. This includes runs over
multiple seeds. The results are saved in the working directory.
:return: None
"""
# go into the first seed directory and read the tasks and dev stages
seed_iter = os.scandir(path)
results = None
for seed_dir in seed_iter:
seed_dir_path = seed_dir.path
if not is_valid_seed_path(seed_dir_path):
continue
seed_dir_path = os.path.join(seed_dir_path, sub_dir)
results = read_tasks_and_dev_stages_from_disk([seed_dir_path])
break
summary = {}
# iterate over all tasks and dev stages
for task_id, task in results.items():
for dev_id, _ in task.items():
summary[(task_id, dev_id)] = summarize_results(
path,
final_task_id=task_id,
final_dev_id=dev_id,
sub_dir=sub_dir,
write_to_file=False,
)


I guess the way to go (if seed dir is still relevant) is to specifically ask for what seed is requested in the function params.

@eddiebergman eddiebergman added the bug Something isn't working label Apr 30, 2024
@eddiebergman
Copy link
Contributor Author

As an additional thing to fix, this function calls read_tasks_and_dev_stages_from_disk which essentially reads all results from disk, uses just the ids it retrieves and the proceeds to call anther function summarize_results which will then load all results again.

@eddiebergman
Copy link
Contributor Author

Last point, nothing in NePS is using this function and there's no publicised user API for this. I guess we'll need to rediscover the point of this feature since it's pretty undocumented anywhere.

My guess it's some sort of convinience feature but I don't feel this provides much benefit and perhaps more confusion than if someone just made a different working_directory

@eddiebergman
Copy link
Contributor Author

More issues with this function, pretty sure this would just not work if subdir was given.

os.path.join(sub_dir, user_prior_dir)

@eddiebergman
Copy link
Contributor Author

eddiebergman commented Apr 30, 2024

What even is a user prior dir... what's meant to be in there that's different from any other run directory?

if user_prior_dir is not None:

@eddiebergman eddiebergman added this to the Runtime milestone Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

1 participant