Skip to content

Feature/add requested vram#23

Merged
MisterArdavan merged 32 commits into
mainfrom
feature/add-requested-vram
Sep 17, 2025
Merged

Feature/add requested vram#23
MisterArdavan merged 32 commits into
mainfrom
feature/add-requested-vram

Conversation

@MisterArdavan

@MisterArdavan MisterArdavan commented Aug 17, 2025

Copy link
Copy Markdown
Collaborator

Adds requested_vram to the metrics calculated for users and PI groups and replaces efficiency and score metrics based on vram_constraint alone.

This branch is created off feature/high-cpu-mem-analysis(#22).

@MisterArdavan MisterArdavan force-pushed the feature/add-requested-vram branch from f585e97 to 2b861be Compare August 20, 2025 12:40
Comment thread notebooks/.gitattributes
*.ipynb filter=strip-notebook-output
# keep the output of the following notebooks when committing
SlurmGPU.ipynb !filter=strip-notebook-output No newline at end of file
*.ipynb filter=strip-notebook-output No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why are we not keeping the SlurmGPU output now?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The list of notebooks to not filter out is handled by the script clean_notebook.sh so this was unnecessary.

from .errors import JobPreprocessingError


class Preprocess:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see the point of making this a class, as it just adds a layer of complication (and again, the other code for queries etc will have to be updated for no reason). The functions work fine, and we don't need to store a state. We could just pass in anonymize into the function if we need to. Also, maybe anonymize is not the best word since it works for replacing user names but it isn't exactly clear that it also removes logs. Maybe add a separate parameter for removing logs, or rename this?

@Espiobest

Espiobest commented Aug 21, 2025

Copy link
Copy Markdown
Collaborator

We probably shouldn't merge breaking changes (like the new parameters in EF and the preprocess class) because every other piece (even if not merged) would need to be changed, and we don't have time to do that. Even reports, which should be one of the main components.

@MisterArdavan MisterArdavan left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I applied the changes I had made in preprocess to match the new functions in utilities. I also added the two analysis notebooks with their outputs anonymized to the list of notebooks that we keep their outputs. The only thing that remains is to add a short project description to the main project README that also references the notebooks.

Comment thread notebooks/.gitattributes
*.ipynb filter=strip-notebook-output
# keep the output of the following notebooks when committing
SlurmGPU.ipynb !filter=strip-notebook-output No newline at end of file
*.ipynb filter=strip-notebook-output No newline at end of file

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The list of notebooks to not filter out is handled by the script clean_notebook.sh so this was unnecessary.

@MisterArdavan MisterArdavan merged commit 36945ba into main Sep 17, 2025
6 checks passed
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.

2 participants