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.

Sign up for GitHub

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

Jump to bottom

Add BinaryOutput #1099

Merged
merged 6 commits into from
May 26, 2023
Merged

Add BinaryOutput #1099

merged 6 commits into from
May 26, 2023

Conversation

edknv
Copy link
Contributor

@edknv edknv commented May 17, 2023

Goals ⚽

This PR introduces the following classes for the pytorch backend:

  • ModelOutput
  • BinaryOutput

Implementation Details 🚧

Testing Details 🔍

Coverage:

Name                                            Stmts   Miss  Cover
-------------------------------------------------------------------
merlin/models/torch/outputs/__init__.py             0      0   100%
merlin/models/torch/outputs/base.py                23      0   100%
merlin/models/torch/outputs/classification.py      16      0   100%
-------------------------------------------------------------------
TOTAL                                              39      0   100%

@edknv edknv requested a review from marcromeyn May 17, 2023 04:34
@edknv edknv self-assigned this May 17, 2023
@edknv edknv added enhancement New feature or request area/pytorch labels May 17, 2023
@edknv edknv added this to the Merlin 23.06 milestone May 17, 2023
@github-actions
Copy link

Documentation preview

https://nvidia-merlin.github.io/models/review/pr-1099

marcromeyn
marcromeyn reviewed May 17, 2023
View reviewed changes
merlin/models/torch/outputs/base.py
self.setup_schema(schema)
self.register_buffer("target", torch.zeros(1, dtype=torch.float32))

def setup_schema(self, schema: Optional[ColumnSchema]):
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we need to make things more explicit and turn this into setup_column_schema?

Copy link
Contributor Author

@edknv edknv May 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I guess we could go either way, but I kept setup_schema in 2385020, since output_schema is a Schema not a ColumnSchema.

@edknv edknv mentioned this pull request May 26, 2023
Merged
@marcromeyn marcromeyn merged commit efaa5c0 into NVIDIA-Merlin:main May 26, 2023
@marcromeyn marcromeyn mentioned this pull request May 30, 2023
Open
26 tasks
@marcromeyn marcromeyn mentioned this pull request Jul 3, 2023
Open
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcromeyn marcromeyn marcromeyn approved these changes

Assignees

@edknv edknv

Labels
area/pytorch enhancement New feature or request
Projects
None yet
Milestone
Merlin 23.06
Development

Successfully merging this pull request may close these issues.
2 participants
@edknv @marcromeyn