Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 6 additions & 0 deletions keras/src/layers/core/dense.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ def __init__(
lora_alpha=None,
**kwargs,
):
if not isinstance(units, int) or units <= 0:
raise ValueError(
"Received an invalid value for `units`, expected a positive integer. "
f"Received: units={units}"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The validation isinstance(units, int) is a bit too strict as it will reject numpy integer types (e.g., numpy.int64), which are commonly used in the scientific Python ecosystem. This can lead to unexpected errors for users.

A more robust approach is to check if units is integer-like by trying to convert it to an int and checking for equality. This correctly handles integers, numpy integers, and integer-like floats (e.g., 2.0), while rejecting non-integer floats and other types like strings.

Suggested change
if not isinstance(units, int) or units <= 0:
raise ValueError(
"Received an invalid value for `units`, expected a positive integer. "
f"Received: units={units}"
)
try:
is_int_like = int(units) == units
except (TypeError, ValueError):
is_int_like = False
if not is_int_like or units <= 0:
raise ValueError(
"Received an invalid value for `units`, expected a positive integer. "
f"Received: units={units}"
)


super().__init__(activity_regularizer=activity_regularizer, **kwargs)
self.units = units
self.activation = activations.get(activation)
Expand Down
16 changes: 16 additions & 0 deletions keras/src/layers/core/dense_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,22 @@ def test_dense_basics(self):
expected_num_losses=2, # we have 2 regularizers.
supports_masking=True,
)

def test_dense_invalid_units_raises(self):
with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(0)

with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(-3)

with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(2.5)

with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(None)

with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense("64")
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

These tests for invalid units values are great. To make the test more concise and easier to extend in the future, you could use parameterized.named_parameters which is already used in this test file.

Suggested change
def test_dense_invalid_units_raises(self):
with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(0)
with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(-3)
with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(2.5)
with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(None)
with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense("64")
@parameterized.named_parameters(
("zero", 0),
("negative", -3),
("float", 2.5),
("none", None),
("string", "64"),
)
def test_dense_invalid_units_raises(self, units):
with self.assertRaisesRegex(ValueError, "positive integer"):
layers.Dense(units)


def test_dense_correctness(self):
# With bias and activation.
Expand Down
Loading