Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/gfdl/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def fit(self, X, Y):
# Y shape: (n_samples, n_classes-1)
if self.reg_alpha is not None and self.reg_alpha < 0.0:
raise ValueError("Negative reg_alpha. Expected range : None or [0.0, inf).")
hidden_layer_sizes = np.asarray(self.hidden_layer_sizes)
hidden_layer_sizes = np.atleast_1d(np.asarray(self.hidden_layer_sizes))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that, here and elsewhere, you can simplify the call to remove np.asarray() since atleast_1d should call something similar for array coercion under the hood.

if hidden_layer_sizes.min() < 1:
raise ValueError("hidden_layer_sizes must be > 0, "
f"got {hidden_layer_sizes}")
Expand Down Expand Up @@ -144,7 +144,7 @@ def partial_fit(self, X, y):
fn = resolve_activation(self.activation)[1]
self._activation_fn = fn
self._N = X.shape[1]
hidden_layer_sizes = np.asarray(self.hidden_layer_sizes)
hidden_layer_sizes = np.atleast_1d(np.asarray(self.hidden_layer_sizes))
self._weight_mode = resolve_weight(self.weight_scheme)

# weights shape: (n_layers,)
Expand Down Expand Up @@ -565,7 +565,7 @@ def fit(self, X, Y):
if self.reg_alpha is not None and self.reg_alpha < 0.0:
raise ValueError("Negative reg_alpha. Expected range : None or [0.0, inf).")

hidden_layer_sizes = np.asarray(self.hidden_layer_sizes)
hidden_layer_sizes = np.atleast_1d(np.asarray(self.hidden_layer_sizes))
if hidden_layer_sizes.min() < 1:
raise ValueError("hidden_layer_sizes must be > 0, "
f"got {hidden_layer_sizes}")
Expand Down Expand Up @@ -632,7 +632,7 @@ def partial_fit(self, X, Y):
fn = resolve_activation(self.activation)[1]
self._activation_fn = fn
self._N = X.shape[1]
hidden_layer_sizes = np.asarray(self.hidden_layer_sizes)
hidden_layer_sizes = np.atleast_1d(np.asarray(self.hidden_layer_sizes))
self._weight_mode = resolve_weight(self.weight_scheme)

self.W_ = []
Expand Down
20 changes: 20 additions & 0 deletions src/gfdl/tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -822,3 +822,23 @@ def test_gh_85_classifiers(hidden_layer_sizes, classifier):
clf = classifier(hidden_layer_sizes=hidden_layer_sizes, seed=0)
with pytest.raises(ValueError, match="must be > 0"):
clf.fit(X, y)

@pytest.mark.parametrize(
"Classifier",
[GFDLClassifier, EnsembleGFDLClassifier],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the target issue concerns GFDLRegressor rather than the classifiers. I don't mind if you want to test/fix them separately (regressor tests go in a separate testing module), but let's not consider the target issue fully closed until the GFDLRegressor is tested for the problem scenario in the matching issue.

)
def test_scalar_hidden_layer_sizes(Classifier):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is helpful to reference the issue you are writing a test for, i.e., # see gh-136

X, y = make_classification(
n_samples=60,
n_features=10,
n_informative=5,
random_state=42,
)

model = Classifier(hidden_layer_sizes=3, seed=0)
model.fit(X, y)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose if we're being really thorough, we should also test partial_fit()


predictions = model.predict(X[:5])
assert predictions.shape == (5,)


Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some ruff linting failures here and elsewhere. You can find these by running ruff check with ruff version 0.16.8 locally.

Loading