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

AggregatingSentenceIterator.addSentencePreProcessor() doesn't actually add #8122

Closed
cowwoc opened this issue Aug 21, 2019 · 1 comment · Fixed by SkymindIO/deeplearning4j#141

Comments

@cowwoc
Copy link

commented Aug 21, 2019

Issue Description

Please describe our issue, along with:

  • expected behavior: Invoking AggregatingSentenceIterator.addSentencePreProcessor() should add multiple pre-processors.
  • encountered behavior: The last pre-processor overwrites the previous values.

This is counter-intuitive given the method name. Either rename the method or support multiple pre-processors.

Version Information

Please indicate relevant versions, including, if relevant:

  • Deeplearning4j version: 1.0.0-beta4
  • Platform information (OS, etc): N/A
  • CUDA version, if used: N/A
  • NVIDIA driver version, if in use: N/A
AlexDBlack added a commit to SkymindIO/deeplearning4j that referenced this issue Aug 21, 2019
eclipse#8122 AggregatingSentenceIterator builder - addSentencePreProc…
…essor -> sentencePreProcessor

Signed-off-by: AlexDBlack <blacka101@gmail.com>
AlexDBlack added a commit to SkymindIO/deeplearning4j that referenced this issue Aug 21, 2019
Various fixes (#141)
* eclipse#8121 CnnSentenceDataSetIterator fixes

Signed-off-by: AlexDBlack <blacka101@gmail.com>

* eclipse#8120 CnnSentenceDataSetIterator.loadSingleSentence no words UX/exception improvement

Signed-off-by: AlexDBlack <blacka101@gmail.com>

* eclipse#8122 AggregatingSentenceIterator builder - addSentencePreProcessor -> sentencePreProcessor

Signed-off-by: AlexDBlack <blacka101@gmail.com>

* eclipse#8082 Arbiter - fix GridSearchCandidateGenerator search size issue

Signed-off-by: AlexDBlack <blacka101@gmail.com>
@AlexDBlack

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2019

Thanks for reporting - fixed here (deprecated method + added better named method), will be merged to eclipse/deeplearning4j master soon.
SkymindIO#141

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.