-
Notifications
You must be signed in to change notification settings - Fork 133
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
ROM Train on DataSet #1718
ROM Train on DataSet #1718
Conversation
self.trainingSet = copy.copy(self._inputToInternal(trainingSet)) | ||
# LEGACY SupervisedLearning (SVL) objects train on dictionaries/matrices | ||
# New SVL can bypass the data manip and use the dataset directly | ||
useDict = self.supervisedContainer[0].needsDictTraining |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the use of the check for legacy training (needsDictTraining)
@@ -131,6 +131,7 @@ def __init__(self): | |||
self.muAndSigmaFeatures = {} # normalization parameters | |||
self.metadataKeys = set() # keys that can be passed to DataObject as meta information | |||
self.metadataParams = {} # indexMap for metadataKeys to pass to a DataObject as meta dimensionality | |||
self.needsDictTraining = True # True if the "train" method expects a dictionary ONLY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the defaulting declaration of the legacy node flag. Existing SVLs will all use dicts to train, but this should be discouraged for future algorithms.
@PaulTalbot-INL @wangcj05 @mandd guys, is this going to happen anytime soon? |
If I recall correctly, the DMDc merge introduced some conflicting mechanics as well as code conflicts, so I haven't returned to try and finish it off. If you need it for something, I can try and fit it in (you know, late evening or early mornings) |
oh got it. Do you have an ETA more and less? |
You know how it is, as soon as someone needs it, it will become a priority. I don't foresee needing it until near end of this FY, for my stuff, if not early next year. |
|
@PaulTalbot-INL news on this? |
I believe all of the critical changes have been ported to other PRs and merged. @dylanjm can you confirm? I can close this if there's nothing else of value to scavenge from it. |
@PaulTalbot-INL Yes, I was able to grab the custom sampler stuff in another PR. I think Yeni has a few changes in her RAVEN which she'll push as a PR eventually. Unless there are other changes, I think the majority of what we needed here has been integrated. |
Pull Request Description
What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)
Addresses #731 for ROMs (not PP).
What are the significant changes in functionality due to this change request?
Allows option to pass training data sets directly to ROM SupervisedLearning algorithms rather than converting everything to dictionaries.
A flag is used to allow the SVL to self-identify whether it needs legacy training (dictionaries) or can handle training via DataSet.
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.
<internalParallel>
to True.raven/tests/framework/user_guide
andraven/docs/workshop
) have been changed, the associated documentation must be reviewed and assured the text matches the example.