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

[FEA]: Update DFPTraining stage to accept both ControlMessage and MultiDFPMessage #1148

Closed
6 of 7 tasks
mdemoret-nv opened this issue Aug 23, 2023 · 2 comments · Fixed by #1155
Closed
6 of 7 tasks
Assignees
Labels
dfp [Workflow] Related to the Digital Fingerprinting (DFP) workflow feature request New feature or request

Comments

@mdemoret-nv
Copy link
Contributor

mdemoret-nv commented Aug 23, 2023

Is this a new feature, an improvement, or a change to existing functionality?

New Feature

How would you describe the priority of this feature request

High

Please provide a clear description of problem this feature solves

Currently, the DFP pipeline is implemented using both Modules and Stages. The modules exclusively work with ControlMessage while the stages work exclusively with MultiDFPMessage. This makes it difficult for users customizing the DFP pipeline to use some of the advantages of ControlMessage if they are using stages. One particular area where ControlMessage would be more beneficial is the training stage to allow using data loaders.

Describe your ideal solution

As a first step, the DFPTraining stage should be updated to accept both ControlMessage and MultiDFPMessage and work with either. If it accepts a ControlMessage it should return a ControlMessage and the same for MultiDFPMessage.

Completion Criteria:

  • The DFPTraining accepts both ControlMessage and MultiDFPMessage
    • The type can be determined in the build phase and remain static
  • The DFPTraining returns a ControlMessage if one is passed in or a MultiAEMessage if a DFPMessageMeta is passed in
  • The DFPTraining stage should always pass the data to AutoEncoder as a data loader (even if its a MultiDFPMessage)
  • The training results should be unchanged

Describe any alternatives you have considered

No response

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I have searched the open feature requests and have found no duplicates for this feature request
@mdemoret-nv mdemoret-nv added feature request New feature or request dfp [Workflow] Related to the Digital Fingerprinting (DFP) workflow labels Aug 23, 2023
@mdemoret-nv mdemoret-nv added this to the 23.11 - DFP Improvements milestone Aug 23, 2023
@mdemoret-nv
Copy link
Contributor Author

Additional context, using a Data loader for AutoEncoder should help with issues: #933, #816 and #724

@drobison00
Copy link
Contributor

Supporting torch datasets on the _fit_centralized path requires a variety of changes; after looking over how we do _fit_distribtued, there are enough similarities in what we're doing and what needs to change in _fit_centralized that it makes sense to combine the two paths. Going to work through this in two phases, one to align _fit_centralized's behavior with _fit_distributed and one to merge the methods and eliminate duplicated code.

rapids-bot bot pushed a commit that referenced this issue Sep 21, 2023
…ges (#1155)

Closes #1148

This addresses relevant issues with 1148, additionally, in order to make the .fit interface work in a more unified way, we've merged and simplified the _fit_centralized and _fit_distributed code paths, and eliminated large portions of dead code.

Authors:
  - Devin Robison (https://github.com/drobison00)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)
  - Christopher Harris (https://github.com/cwharris)

URL: #1155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dfp [Workflow] Related to the Digital Fingerprinting (DFP) workflow feature request New feature or request
Projects
Status: Done
2 participants