396 convert regressors to num power 1#416
Conversation
| $this->holdOut = $holdOut; | ||
| $this->costFn = $costFn ?? new LeastSquares(); | ||
| $this->metric = $metric ?? new RMSE(); | ||
| $this->packSamples = $packSamples; |
There was a problem hiding this comment.
Can we autodetect whether we need to pack samples or not? If so I'd prefer we did that instead of passing it in at the constructor. What happens if someone passes in packed samples and then unpacked samples after that?
There was a problem hiding this comment.
Refactored with auto-detection of arrays, that need pack operation - see array_pack() function
| public function featureImportances() : array | ||
| { | ||
| if (!isset($this->ensemble, $this->featureCount)) { | ||
| if (!$this->ensemble || !$this->featureCount) { |
There was a problem hiding this comment.
This new lines is not the same as the old logic. Why did you change it?
| $this->network = new FeedForward( | ||
| new Placeholder1D($dataset->numFeatures()), | ||
| [new Dense(1, $this->l2Penalty, true, new Xavier2())], | ||
| [new Dense(1, $this->l2Penalty, true, new XavierUniform())], |
There was a problem hiding this comment.
Since XavierUniform implements Xavier1 initialization, this change is not equivalent to the old line. We'll need to implement Xavier2Uniform and rename XavierUniform to Xavier1Uniform.
| $this->network = new FeedForward( | ||
| new Placeholder1D($dataset->numFeatures()), | ||
| [new Dense(count($classes), $this->l2Penalty, true, new Xavier1())], | ||
| [new Dense(count($classes), $this->l2Penalty, true, new XavierNormal())], |
There was a problem hiding this comment.
XavierNormal is not equivalent to Xavier1 which samples from a uniform distribution.
| $hiddenLayers = $this->hiddenLayers; | ||
|
|
||
| $hiddenLayers[] = new Dense(count($classes), 0.0, true, new Xavier1()); | ||
| $hiddenLayers[] = new Dense(count($classes), 0.0, true, new XavierNormal()); |
There was a problem hiding this comment.
XavierNormal is not equivalent to Xavier1 which samples from a uniform distribution.
| $this->network = new FeedForward( | ||
| new Placeholder1D($dataset->numFeatures()), | ||
| [new Dense(1, $this->l2Penalty, true, new Xavier1())], | ||
| [new Dense(1, $this->l2Penalty, true, new XavierNormal())], |
There was a problem hiding this comment.
XavierNormal is not equivalent to Xavier1 which samples from a uniform distribution.
| $this->validateFanInFanOut(fanIn: $fanIn, fanOut: $fanOut); | ||
|
|
||
| // Xavier-2 uses fourth-root scaling instead of standard square-root Xavier 1 scaling. | ||
| $limit = (6.0 / ($fanOut + $fanIn)) ** 0.25; |
There was a problem hiding this comment.
Nice, yeah Xavier2 uses that fourth root instead of square root.
There was a problem hiding this comment.
Ok Xavier2 Uniform looks good. Did you want to add Xavier2 Normal just to match the Xavier1 Normal you added?
Nice work regarding array_pack, I like the idea of building the optimization into the function itself. It handles the case of a semi-packed array, an edge case, but bound to happen to someone - so this approach solves that.
I'm asking about the need for depth arguments in the comments. Let me know what you think.
| * @param int $maxDepth | ||
| * @return array<mixed> | ||
| */ | ||
| function array_pack(array $samples, int $depth = 0, int $maxDepth = 100) : array |
There was a problem hiding this comment.
Do we need the $depth and $maxDepth arguments?
"PHP arrays are finite trees (no circular references), so infinite recursion can't happen."
Do we need these arguments for anything beyond preventing infinite recursion? If not, then let's simplify the API. Example (untested) ...
function array_pack(array $samples): array
{
if (!array_is_list($samples)) {
$samples = array_values($samples);
}
return array_map(function ($item) {
if (is_array($item)) {
if (!array_is_list($item)) {
$item = array_values($item);
}
return array_pack($item);
}
return $item;
}, $samples;
}2 participants
No description provided.