Skip to content

Conversation

@tamarl08
Copy link
Contributor

Metrics are logged to different sections according to the context (rsmtool, rsmeval etc.). That way they appear separately on the W&B run and do not overwrite when more than one module log the same metrics.
All reports are logged to a reports section.

This PR also updates the RTD configuration to meet the new format of the build section.

@tamarl08 tamarl08 linked an issue Sep 27, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
rsmtool/analyzer.py 99.16% <100.00%> (ø)
rsmtool/rsmcompare.py 95.00% <100.00%> (ø)
rsmtool/rsmeval.py 97.89% <100.00%> (ø)
rsmtool/rsmexplain.py 92.15% <100.00%> (ø)
rsmtool/rsmpredict.py 99.39% <100.00%> (ø)
rsmtool/rsmsummarize.py 97.84% <100.00%> (ø)
rsmtool/rsmtool.py 99.21% <100.00%> (ø)
rsmtool/rsmxval.py 98.21% <100.00%> (+0.03%) ⬆️
rsmtool/utils/wandb.py 100.00% <100.00%> (ø)
rsmtool/writer.py 96.29% <100.00%> (+0.06%) ⬆️

📢 Thoughts on this report? Let us know!.

@mulhod
Copy link
Contributor

mulhod commented Sep 28, 2023

I'd like to try these changes in a real scenario. Perhaps we can work together to do that, but it will take some work to do before this is a package that can be installed.

@desilinguist
Copy link
Collaborator

I'd like to try these changes in a real scenario. Perhaps we can work together to do that, but it will take some work to do before this is a package that can be installed.

@tamarl08 let's build a conda package and send the tarball to @mulhod, he can install it locally and test?

Co-authored-by: Matt Mulholland <mmulholland@ets.org>
@desilinguist
Copy link
Collaborator

@tamarl08 I am not seeing the warnings in readthedocs builds anymore, are you? @mulhod I am assuming this work for you as intended now? I am approving based on the assumption since I haven't tested with W&B myself.

Copy link
Collaborator

@desilinguist desilinguist left a comment

Choose a reason for hiding this comment

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

Actually, now that I think about it, we should mention the section stuff in documentation somewhere right?

Copy link
Collaborator

@desilinguist desilinguist left a comment

Choose a reason for hiding this comment

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

LGTM!

@mulhod mulhod self-requested a review October 3, 2023 14:00
@tamarl08 tamarl08 merged commit 2180ca4 into main Oct 3, 2023
@delete-merged-branch delete-merged-branch bot deleted the wandb-sections branch October 3, 2023 14:22
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.

Update ReadTheDocs config

4 participants