Skip to content

Add magnet logger utility#62

Open
b-fenelon wants to merge 1 commit into
mainfrom
evaluation-log
Open

Add magnet logger utility#62
b-fenelon wants to merge 1 commit into
mainfrom
evaluation-log

Conversation

@b-fenelon

Copy link
Copy Markdown
Contributor

Initial attempt at configuring a magnet-level logger by replacing the print statements in evaluation.py with loguru statements. The setup function is pulled out from download_helm_results.py with an additional option for writing out the log (to replace manual copying for aiq-evaluations).

For evaluation, the logger is setup at the evaluate stage to capture the card run directory as its output destination. This logic can happen sooner to collect the card hash and appropriately start the logger sooner. A verbose flag is added to magnet evaluate to raise the logging level to DEBUG, but this has limited usage currently.

@b-fenelon
b-fenelon requested review from Erotemic and dmjoy June 12, 2026 11:57
@dmjoy

dmjoy commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

This looks like a good start to me. Though the JHU card I was testing this with was printing stuff out that wasn't getting captured by this logger (and thus not ending up in the saved log file). Also the log output file has quite a bit of extra info on each line that we might not need (e.g. what file and line the log is coming from). I have a slight preference for just a raw log output saved out if possible. I did something like this with the ALIGN system code where the log printed to stdout has all of the formatting etc. but there's also a "raw" log file saved: https://github.com/ITM-Kitware/align-system/blob/main/align_system/cli/run_align_system.py#L48

Would also be curious to hear Jon's thoughts on all of this though.

@Erotemic

Copy link
Copy Markdown
Contributor

This is the right direction to go.

Something to keep in mind: sequential logging statements are not guaranteed to happen in order if concurrency is happening. This isn't a huge concern here, but I think a hygiene thing we can take care of is instead of printing out logically grouped "blocks" of text line by line, construct the block and then log it out all at once. That way the entire "STATUS" claim is guaranteed to be grouped together. (And it also only has one logger header text, which is the main reason I bring this up)

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.

3 participants