Skip to content

Fix --lb-mode=Memory #275

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

Merged
merged 14 commits into from
May 20, 2025
Merged

Fix --lb-mode=Memory #275

merged 14 commits into from
May 20, 2025

Conversation

juanjosegarcan
Copy link
Contributor

Context

lb-mode Memory is failing when no cell_memory_usage.json or memory_per_metype.json commoning from dry_run are present.

Fix #243

Scope

Add dry_run memory files exists check and throw an error if not informing the user he must dry_run first
Add test for this situation

Testing

Extent tests/unit/test_dry_run.py

Review

  • PR description is complete
  • Coding style (imports, function length, New functions, classes or files) are good
  • Unit/Scientific test added
  • Updated Readme, in-code, developer documentation

@juanjosegarcan juanjosegarcan self-assigned this May 16, 2025
@juanjosegarcan juanjosegarcan requested review from WeinaJi and mgeplf May 19, 2025 06:43
@juanjosegarcan juanjosegarcan requested a review from mgeplf May 19, 2025 07:22
@juanjosegarcan juanjosegarcan requested a review from mgeplf May 19, 2025 13:20
@juanjosegarcan juanjosegarcan force-pushed the lb_mode_memory_error branch from d0ed1e0 to 568c6e4 Compare May 20, 2025 09:56
@juanjosegarcan juanjosegarcan force-pushed the lb_mode_memory_error branch from 568c6e4 to 5e96176 Compare May 20, 2025 09:57
@cattabiani
Copy link
Contributor

we should find a away to not make coveralls mark it as x if we have a ... 0.01% decrease in coverage tho

@juanjosegarcan juanjosegarcan requested a review from mgeplf May 20, 2025 10:28
Copy link
Contributor

@cattabiani cattabiani left a comment

Choose a reason for hiding this comment

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

I just have a suggestion but I think it can be helpful

@juanjosegarcan juanjosegarcan requested a review from cattabiani May 20, 2025 13:30
Copy link
Contributor

@cattabiani cattabiani left a comment

Choose a reason for hiding this comment

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

1 small thing

Copy link
Contributor

@cattabiani cattabiani left a comment

Choose a reason for hiding this comment

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

LGTM

@juanjosegarcan juanjosegarcan merged commit d29a6cf into main May 20, 2025
16 of 19 checks passed
@juanjosegarcan juanjosegarcan deleted the lb_mode_memory_error branch May 20, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--lb-mode=Memory doesn't work
4 participants