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

Third order projection progress tracker #158

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

niklundgren
Copy link
Collaborator

Motivation:
In a recent commit pulled to main, the logging information was removed from the third order projections. However, these projections make up the bulk of CPU hours used when running heat transport simulations with kALDo. Therefore, logging the progress of these projections allows users to benchmark performance and predict CPU requirements for a given calculation.

Solution:
I reimplemented the progress logging in its original form in two commits (in project_crystal and project_amorphous) I then added two commits that improved the readability of the loggers by using formatted string literals which also reduces the number of function calls.
- function call removed: np.round()

I also added logic to only output this information every 200 modes in the amorphous (consistent with the frequency of the old logger in the project_crystal function) at the cost of one boolean evaluation per mode, but saving two logging calls on the 199 other passes.

Commit that removed the logger SHA: 925eda3 link
Merged in PR #147

…s aware of the progress of their third order projections. This was missed in reviews but @ZKC19940412 and I feel this was a needless removal. Here I am restoring the logger.
…he logger as written in it's original form for the project_crystal function
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.26%. Comparing base (6c5c43e) to head (655bbc8).

Additional details and impacted files

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.

1 participant