Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/16194
@sethah +1, I prefer not to do any update. I think this isuue will be
resolved by the way after some refactor on SharedParam.
---
If your project is set up for it, you can reply to this email
Github user sethah commented on the issue:
https://github.com/apache/spark/pull/16194
I lean towards doing nothing, unless we can find a solution that is both
generic AND lists all/only relevant information.
---
If your project is set up for it, you can reply to this email and have
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16194
To move towards a resolution, I'd either implement one generic `toString`
that enumerates params, or not do anything at this stage. Up to you
@zhengruifeng
---
If your project is set up for it,
Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/16194
@sethah No. Just for the convenience in repl. It's somewhat confusing when
the model's desc is just the uid of its trainer when I work in repl.
---
If your project is set up for it, you can
Github user sethah commented on the issue:
https://github.com/apache/spark/pull/16194
One problem I see is that some models have relevant information stored in
class members instead of params. LogisticRegression for example would probably
list the number of classes and the number of
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16194
It still may be better than manually implementing so many toString methods.
The worst case is that this carries some extra output.
---
If your project is set up for it, you can reply to this email
Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/16194
Good point. This make me think of the usage of `instr.logParams(params:
_*)` in training instrumentation [https://github.com/apache/spark/pull/15671].
IMIO, params are copied from its
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16194
Oh that's a good point, you can enumerate the params generically? that
makes a lot more sense.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user hhbyyh commented on the issue:
https://github.com/apache/spark/pull/16194
Sorry but I would still strongly prefer to override the toString() in Model
or Params. The default implementation can provide more information about the
params. And only in a few cases you need to
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16194
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16194
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70057/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16194
**[Test build #70057 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70057/consoleFull)**
for PR 16194 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16194
**[Test build #70057 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70057/consoleFull)**
for PR 16194 at commit
Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/16194
@srowen Ok, I will update the version. @hhbyyh as Owen said, if we add a
`toString` in class `Model`, we still need to override it in most algs if we
want to output some specific info: for
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16194
Yes, it would be nicer to just have a default like `ClassName(uid=...)` at
least. That however would not include the class-specific details like number of
layers. I am neutral on whether that's so
Github user hhbyyh commented on the issue:
https://github.com/apache/spark/pull/16194
I haven't really checked about it, but is it possible to just provide the
default `toString` implementation in some upstream abstract class, like Model?
And we can do the similar thing for
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16194
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16194
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69789/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16194
**[Test build #69789 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69789/consoleFull)**
for PR 16194 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16194
**[Test build #69789 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69789/consoleFull)**
for PR 16194 at commit
20 matches
Mail list logo
