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

Fixed Reporting #541

Merged
merged 8 commits into from
Nov 11, 2018
Merged

Fixed Reporting #541

merged 8 commits into from
Nov 11, 2018

Conversation

philip30
Copy link
Contributor

@philip30 philip30 commented Nov 1, 2018

I fixed reporting so that compute_report is determined if there is a reporter being defined, rather than making a separate flag in exp_global. The fix includes if there are multiples eval_tasks that want to write reports to different places.

Having compute_report on exp_global is somewhat confusing, and the report should just be computed as needed (if the Reporter is defined).

@philip30
Copy link
Contributor Author

philip30 commented Nov 6, 2018

+1 metric to calculate segmentation F-measure.

@philip30
Copy link
Contributor Author

philip30 commented Nov 7, 2018

Any comment @msperber?

@msperber
Copy link
Contributor

msperber commented Nov 7, 2018

I've been a bit busy, but I will take a look within the next days!

Copy link
Contributor

@msperber msperber left a comment

Choose a reason for hiding this comment

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

I took a look, I think this is great and much more intuitive to use than before. Besides the inline comments, I noticed that the documentation still explains reporting the old way, could you adjust that?

xnmt/eval/tasks.py Outdated Show resolved Hide resolved
xnmt/inferences.py Outdated Show resolved Hide resolved
@philip30
Copy link
Contributor Author

I have reflected all the comments, now I am merging. Thanks, @msperber !

@philip30 philip30 merged commit 8a8f0cd into master Nov 11, 2018
@philip30 philip30 deleted the reinf_debug branch November 11, 2018 23:24
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