From 20dd14b6a04bf1333358761445253eda3e49e778 Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Sun, 11 Feb 2024 16:34:54 -0500 Subject: [PATCH 01/11] add stream_data --- mix.exs | 1 + mix.lock | 1 + 2 files changed, 2 insertions(+) diff --git a/mix.exs b/mix.exs index 9f05b423a..7908b5a04 100644 --- a/mix.exs +++ b/mix.exs @@ -50,6 +50,7 @@ defmodule Explorer.MixProject do ## Test {:bypass, "~> 2.1", only: :test}, + {:stream_data, "~> 0.6", only: :test}, ## Dev {:ex_doc, "~> 0.24", only: :dev}, diff --git a/mix.lock b/mix.lock index accd491e2..3c9af8556 100644 --- a/mix.lock +++ b/mix.lock @@ -30,6 +30,7 @@ "rustler": {:hex, :rustler, "0.29.1", "880f20ae3027bd7945def6cea767f5257bc926f33ff50c0d5d5a5315883c084d", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:toml, "~> 0.6", [hex: :toml, repo: "hexpm", optional: false]}], "hexpm", "109497d701861bfcd26eb8f5801fe327a8eef304f56a5b63ef61151ff44ac9b6"}, "rustler_precompiled": {:hex, :rustler_precompiled, "0.7.1", "ecadf02cc59a0eccbaed6c1937303a5827fbcf60010c541595e6d3747d3d0f9f", [:mix], [{:castore, "~> 0.1 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: false]}, {:rustler, "~> 0.23", [hex: :rustler, repo: "hexpm", optional: true]}], "hexpm", "b9e4657b99a1483ea31502e1d58c464bedebe9028808eda45c3a429af4550c66"}, "statistex": {:hex, :statistex, "1.0.0", "f3dc93f3c0c6c92e5f291704cf62b99b553253d7969e9a5fa713e5481cd858a5", [:mix], [], "hexpm", "ff9d8bee7035028ab4742ff52fc80a2aa35cece833cf5319009b52f1b5a86c27"}, + "stream_data": {:hex, :stream_data, "0.6.0", "e87a9a79d7ec23d10ff83eb025141ef4915eeb09d4491f79e52f2562b73e5f47", [:mix], [], "hexpm", "b92b5031b650ca480ced047578f1d57ea6dd563f5b57464ad274718c9c29501c"}, "table": {:hex, :table, "0.1.2", "87ad1125f5b70c5dea0307aa633194083eb5182ec537efc94e96af08937e14a8", [:mix], [], "hexpm", "7e99bc7efef806315c7e65640724bf165c3061cdc5d854060f74468367065029"}, "table_rex": {:hex, :table_rex, "4.0.0", "3c613a68ebdc6d4d1e731bc973c233500974ec3993c99fcdabb210407b90959b", [:mix], [], "hexpm", "c35c4d5612ca49ebb0344ea10387da4d2afe278387d4019e4d8111e815df8f55"}, "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"}, From b5f98c0f818c6ec09fb83463dd2f2f9542fa360f Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Sun, 11 Feb 2024 16:43:21 -0500 Subject: [PATCH 02/11] first set of properties (includes a panic) --- .../series/inferred_dtype_property_test.exs | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 test/explorer/series/inferred_dtype_property_test.exs diff --git a/test/explorer/series/inferred_dtype_property_test.exs b/test/explorer/series/inferred_dtype_property_test.exs new file mode 100644 index 000000000..28e901d41 --- /dev/null +++ b/test/explorer/series/inferred_dtype_property_test.exs @@ -0,0 +1,115 @@ +defmodule Explorer.Series.InferredDtypePropertyTest do + @moduledoc """ + Property tests for checking the inferred dtype logic when the dtype isn't + specified in `Explorer.Series.from_list/1`. + """ + use ExUnit.Case, async: true + use ExUnitProperties + + alias Explorer.Series + + @moduletag timeout: :infinity + + # Not working! Results in a panic. + property "specific example generated by `{:list, {:list, {:s, 64}}}` test below" do + Series.from_list([[], [[]], [[0], [], []]]) + end + + # Not working! Results in a panic. + property "{:list, {:list, {:s, 64}}}" do + dtype = {:list, {:list, {:s, 64}}} + + check all( + series <- series_of_dtype_generator(dtype), + initial_seed: 0, + max_runs: 3 + ) do + assert series |> Series.dtype() |> sub_dtype_of?(dtype) + end + end + + property "inferred dtype should always be a sub-dtype" do + check all( + {dtype, series} <- dtype_series_tuple_generator(), + max_run_time: 60_000 + ) do + assert series |> Series.dtype() |> sub_dtype_of?(dtype) + end + end + + defp dtype_generator do + scalar_dtype_generator = StreamData.constant({:s, 64}) + + # WARNING! This method of generation is slightly flawed. The struct + # generator could result a list of tuples with duplicate keys. + dtype_generator = + StreamData.tree(scalar_dtype_generator, fn generator -> + StreamData.one_of([ + StreamData.tuple({ + StreamData.constant(:list), + generator + }), + StreamData.tuple({ + StreamData.constant(:struct), + StreamData.nonempty( + StreamData.list_of( + StreamData.tuple({ + StreamData.string(:alphanumeric, min_length: 1), + generator + }) + ) + ) + }) + ]) + end) + + dtype_generator + end + + defp dtype_series_tuple_generator() do + StreamData.bind(dtype_generator(), fn dtype -> + series_value_generator = build_series_value_generator(dtype) + + StreamData.bind(StreamData.list_of(series_value_generator), fn series_values -> + StreamData.constant({dtype, Explorer.Series.from_list(series_values)}) + end) + end) + end + + defp series_of_dtype_generator(dtype) do + series_value_generator = build_series_value_generator(dtype) + + StreamData.bind(StreamData.list_of(series_value_generator), fn series_values -> + StreamData.constant(Explorer.Series.from_list(series_values)) + end) + end + + defp build_series_value_generator({:s, 64}), + do: StreamData.integer() + + defp build_series_value_generator({:list, dtype}), + do: StreamData.list_of(build_series_value_generator(dtype)) + + defp build_series_value_generator({:struct, keyword_of_dtypes}) do + keyword_of_dtypes + |> Map.new(fn {key, dtype} -> {key, build_series_value_generator(dtype)} end) + |> StreamData.fixed_map() + end + + defp sub_dtype_of?(x, x), do: true + defp sub_dtype_of?(:null, _), do: true + defp sub_dtype_of?({:list, sub_dtype}, {:list, dtype}), do: sub_dtype_of?(sub_dtype, dtype) + + defp sub_dtype_of?({:struct, sub_dtype_keyword}, {:struct, dtype_keyword}) + when is_list(sub_dtype_keyword) and is_list(dtype_keyword) do + # Note: the need to sort here indicates we may want to normalize the result + # of `Series.dtype/1`. + Enum.sort(sub_dtype_keyword) + |> Enum.zip(Enum.sort(dtype_keyword)) + |> Enum.all?(fn {{sub_key, sub_value}, {key, value}} -> + sub_key == key and sub_dtype_of?(sub_value, value) + end) + end + + defp sub_dtype_of?(_sub_dtype, _dtype), do: false +end From ab41fcd9bb4937119780d88f5492dc3817834b55 Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Tue, 13 Feb 2024 13:24:01 -0500 Subject: [PATCH 03/11] exclude property tests by default --- test/test_helper.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_helper.exs b/test/test_helper.exs index 4e30d229b..3800bfa64 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -43,4 +43,4 @@ defmodule Explorer.IOHelpers do end end -ExUnit.start(exclude: :cloud_integration) +ExUnit.start(exclude: [:cloud_integration, :property]) From d4ba6f04d170d21c0dcabc0d90d4e2729a19c416 Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Tue, 13 Feb 2024 13:24:31 -0500 Subject: [PATCH 04/11] run property tests in CI (hopefully, it's untested atm) --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93193d89a..cae779335 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,11 +49,14 @@ jobs: run: mix test --warnings-as-errors - name: Compile once again but without optional deps run: mix compile --force --warnings-as-errors --no-optional-deps - - name: Run cloud integration tests run: | mix localstack.setup mix test --only cloud_integration + - name: Run property tests + run: | + mix localstack.setup + mix test --only property format: runs-on: ubuntu-latest From 350450769460464338803b9aa7b0b4a2b417353f Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Tue, 13 Feb 2024 13:25:25 -0500 Subject: [PATCH 05/11] remove cases now in main --- .../series/inferred_dtype_property_test.exs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/test/explorer/series/inferred_dtype_property_test.exs b/test/explorer/series/inferred_dtype_property_test.exs index 28e901d41..d277a30be 100644 --- a/test/explorer/series/inferred_dtype_property_test.exs +++ b/test/explorer/series/inferred_dtype_property_test.exs @@ -10,24 +10,6 @@ defmodule Explorer.Series.InferredDtypePropertyTest do @moduletag timeout: :infinity - # Not working! Results in a panic. - property "specific example generated by `{:list, {:list, {:s, 64}}}` test below" do - Series.from_list([[], [[]], [[0], [], []]]) - end - - # Not working! Results in a panic. - property "{:list, {:list, {:s, 64}}}" do - dtype = {:list, {:list, {:s, 64}}} - - check all( - series <- series_of_dtype_generator(dtype), - initial_seed: 0, - max_runs: 3 - ) do - assert series |> Series.dtype() |> sub_dtype_of?(dtype) - end - end - property "inferred dtype should always be a sub-dtype" do check all( {dtype, series} <- dtype_series_tuple_generator(), From 39ae92ea2d6c089497289c381e17fa611a5e248f Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Wed, 14 Feb 2024 11:38:45 -0500 Subject: [PATCH 06/11] fix duplicate keys issue --- .../series/inferred_dtype_property_test.exs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/test/explorer/series/inferred_dtype_property_test.exs b/test/explorer/series/inferred_dtype_property_test.exs index d277a30be..f36e66e53 100644 --- a/test/explorer/series/inferred_dtype_property_test.exs +++ b/test/explorer/series/inferred_dtype_property_test.exs @@ -22,8 +22,9 @@ defmodule Explorer.Series.InferredDtypePropertyTest do defp dtype_generator do scalar_dtype_generator = StreamData.constant({:s, 64}) - # WARNING! This method of generation is slightly flawed. The struct - # generator could result a list of tuples with duplicate keys. + # We don't need complicated keys: single letter strings should suffice. + key_generator = StreamData.string(?a..?z, min_length: 1, max_length: 1) + dtype_generator = StreamData.tree(scalar_dtype_generator, fn generator -> StreamData.one_of([ @@ -33,13 +34,10 @@ defmodule Explorer.Series.InferredDtypePropertyTest do }), StreamData.tuple({ StreamData.constant(:struct), - StreamData.nonempty( - StreamData.list_of( - StreamData.tuple({ - StreamData.string(:alphanumeric, min_length: 1), - generator - }) - ) + StreamData.map( + StreamData.nonempty(StreamData.map_of(key_generator, generator, max_length: 3)), + # Building the list from a map then ensures unique keys. + &Enum.to_list/1 ) }) ]) From 7ab53691702e0cc5680ed8524f3535fad9d624e5 Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Wed, 14 Feb 2024 11:39:01 -0500 Subject: [PATCH 07/11] cap max list size to 3; notes --- .../series/inferred_dtype_property_test.exs | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/test/explorer/series/inferred_dtype_property_test.exs b/test/explorer/series/inferred_dtype_property_test.exs index f36e66e53..9f4b9dbfa 100644 --- a/test/explorer/series/inferred_dtype_property_test.exs +++ b/test/explorer/series/inferred_dtype_property_test.exs @@ -2,6 +2,12 @@ defmodule Explorer.Series.InferredDtypePropertyTest do @moduledoc """ Property tests for checking the inferred dtype logic when the dtype isn't specified in `Explorer.Series.from_list/1`. + + ## Notes + + * A maximum of 3 used quite a bit. This is intentional. Usually issues stem + from empty lists, not really long lists. By keeping lists small, we can + iterate much quicker through the solution space. """ use ExUnit.Case, async: true use ExUnitProperties @@ -12,8 +18,10 @@ defmodule Explorer.Series.InferredDtypePropertyTest do property "inferred dtype should always be a sub-dtype" do check all( - {dtype, series} <- dtype_series_tuple_generator(), - max_run_time: 60_000 + dtype <- dtype_generator(), + series <- series_of_dtype_generator(dtype), + max_run_time: 60_000, + max_runs: 10_000 ) do assert series |> Series.dtype() |> sub_dtype_of?(dtype) end @@ -46,20 +54,10 @@ defmodule Explorer.Series.InferredDtypePropertyTest do dtype_generator end - defp dtype_series_tuple_generator() do - StreamData.bind(dtype_generator(), fn dtype -> - series_value_generator = build_series_value_generator(dtype) - - StreamData.bind(StreamData.list_of(series_value_generator), fn series_values -> - StreamData.constant({dtype, Explorer.Series.from_list(series_values)}) - end) - end) - end - defp series_of_dtype_generator(dtype) do series_value_generator = build_series_value_generator(dtype) - StreamData.bind(StreamData.list_of(series_value_generator), fn series_values -> + StreamData.bind(StreamData.list_of(series_value_generator, max_length: 3), fn series_values -> StreamData.constant(Explorer.Series.from_list(series_values)) end) end @@ -68,7 +66,7 @@ defmodule Explorer.Series.InferredDtypePropertyTest do do: StreamData.integer() defp build_series_value_generator({:list, dtype}), - do: StreamData.list_of(build_series_value_generator(dtype)) + do: StreamData.list_of(build_series_value_generator(dtype), max_length: 3) defp build_series_value_generator({:struct, keyword_of_dtypes}) do keyword_of_dtypes @@ -76,6 +74,9 @@ defmodule Explorer.Series.InferredDtypePropertyTest do |> StreamData.fixed_map() end + # The idea behind a "sub" dtype is that in the dtype tree, you can replace + # any subtree with `:null` and it's still valid. This is to deal with empty + # lists where we can't reasonably infer the dtype of a list with no elements. defp sub_dtype_of?(x, x), do: true defp sub_dtype_of?(:null, _), do: true defp sub_dtype_of?({:list, sub_dtype}, {:list, dtype}), do: sub_dtype_of?(sub_dtype, dtype) From 0916ba03adc49cf7375e0db31c110946d248d746 Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Wed, 14 Feb 2024 11:40:32 -0500 Subject: [PATCH 08/11] fix: wrong word --- test/explorer/series/inferred_dtype_property_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/explorer/series/inferred_dtype_property_test.exs b/test/explorer/series/inferred_dtype_property_test.exs index 9f4b9dbfa..c1d997613 100644 --- a/test/explorer/series/inferred_dtype_property_test.exs +++ b/test/explorer/series/inferred_dtype_property_test.exs @@ -7,7 +7,7 @@ defmodule Explorer.Series.InferredDtypePropertyTest do * A maximum of 3 used quite a bit. This is intentional. Usually issues stem from empty lists, not really long lists. By keeping lists small, we can - iterate much quicker through the solution space. + iterate much quicker through the input space. """ use ExUnit.Case, async: true use ExUnitProperties From 1c5d3efd98c646a31336307920d183d42c89859d Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Wed, 14 Feb 2024 11:47:53 -0500 Subject: [PATCH 09/11] fix bad copy/paste; also reorder steps --- .github/workflows/ci.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cae779335..cc333f1fd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,16 +47,15 @@ jobs: - run: mix deps.compile - name: Run tests run: mix test --warnings-as-errors + - name: Run property tests + run: | + mix test --only property --warnings-as-errors - name: Compile once again but without optional deps run: mix compile --force --warnings-as-errors --no-optional-deps - name: Run cloud integration tests run: | mix localstack.setup mix test --only cloud_integration - - name: Run property tests - run: | - mix localstack.setup - mix test --only property format: runs-on: ubuntu-latest From 2bf741b780d49f4c432fbd55a4facc8a1974d06d Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Wed, 14 Feb 2024 12:02:19 -0500 Subject: [PATCH 10/11] fail when lengths don't match (zip may miss this) --- .../series/inferred_dtype_property_test.exs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/explorer/series/inferred_dtype_property_test.exs b/test/explorer/series/inferred_dtype_property_test.exs index c1d997613..658adcf61 100644 --- a/test/explorer/series/inferred_dtype_property_test.exs +++ b/test/explorer/series/inferred_dtype_property_test.exs @@ -83,13 +83,17 @@ defmodule Explorer.Series.InferredDtypePropertyTest do defp sub_dtype_of?({:struct, sub_dtype_keyword}, {:struct, dtype_keyword}) when is_list(sub_dtype_keyword) and is_list(dtype_keyword) do - # Note: the need to sort here indicates we may want to normalize the result - # of `Series.dtype/1`. - Enum.sort(sub_dtype_keyword) - |> Enum.zip(Enum.sort(dtype_keyword)) - |> Enum.all?(fn {{sub_key, sub_value}, {key, value}} -> - sub_key == key and sub_dtype_of?(sub_value, value) - end) + if length(sub_dtype_keyword) != length(dtype_keyword) do + false + else + # Note: the need to sort here indicates we may want to normalize the result + # of `Series.dtype/1`. + Enum.sort(sub_dtype_keyword) + |> Enum.zip(Enum.sort(dtype_keyword)) + |> Enum.all?(fn {{sub_key, sub_value}, {key, value}} -> + sub_key == key and sub_dtype_of?(sub_value, value) + end) + end end defp sub_dtype_of?(_sub_dtype, _dtype), do: false From a78b283acbc7bdf495e3ae13b287033a7cbe21db Mon Sep 17 00:00:00 2001 From: William Lanchantin Date: Wed, 14 Feb 2024 12:46:50 -0500 Subject: [PATCH 11/11] import StreamData for readability --- .../series/inferred_dtype_property_test.exs | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/test/explorer/series/inferred_dtype_property_test.exs b/test/explorer/series/inferred_dtype_property_test.exs index 658adcf61..877479154 100644 --- a/test/explorer/series/inferred_dtype_property_test.exs +++ b/test/explorer/series/inferred_dtype_property_test.exs @@ -12,6 +12,8 @@ defmodule Explorer.Series.InferredDtypePropertyTest do use ExUnit.Case, async: true use ExUnitProperties + import StreamData + alias Explorer.Series @moduletag timeout: :infinity @@ -28,26 +30,20 @@ defmodule Explorer.Series.InferredDtypePropertyTest do end defp dtype_generator do - scalar_dtype_generator = StreamData.constant({:s, 64}) + scalar_dtype_generator = constant({:s, 64}) # We don't need complicated keys: single letter strings should suffice. - key_generator = StreamData.string(?a..?z, min_length: 1, max_length: 1) + key_generator = string(?a..?z, min_length: 1, max_length: 1) dtype_generator = - StreamData.tree(scalar_dtype_generator, fn generator -> - StreamData.one_of([ - StreamData.tuple({ - StreamData.constant(:list), - generator - }), - StreamData.tuple({ - StreamData.constant(:struct), - StreamData.map( - StreamData.nonempty(StreamData.map_of(key_generator, generator, max_length: 3)), - # Building the list from a map then ensures unique keys. - &Enum.to_list/1 - ) - }) + tree(scalar_dtype_generator, fn generator -> + # Building the keyword list from a map ensures unique keys. + keyword_generator = + map(nonempty(map_of(key_generator, generator, max_length: 3)), &Enum.to_list/1) + + one_of([ + tuple({constant(:list), generator}), + tuple({constant(:struct), keyword_generator}) ]) end) @@ -57,21 +53,21 @@ defmodule Explorer.Series.InferredDtypePropertyTest do defp series_of_dtype_generator(dtype) do series_value_generator = build_series_value_generator(dtype) - StreamData.bind(StreamData.list_of(series_value_generator, max_length: 3), fn series_values -> - StreamData.constant(Explorer.Series.from_list(series_values)) + bind(list_of(series_value_generator, max_length: 3), fn series_values -> + constant(Explorer.Series.from_list(series_values)) end) end defp build_series_value_generator({:s, 64}), - do: StreamData.integer() + do: integer() defp build_series_value_generator({:list, dtype}), - do: StreamData.list_of(build_series_value_generator(dtype), max_length: 3) + do: list_of(build_series_value_generator(dtype), max_length: 3) defp build_series_value_generator({:struct, keyword_of_dtypes}) do keyword_of_dtypes |> Map.new(fn {key, dtype} -> {key, build_series_value_generator(dtype)} end) - |> StreamData.fixed_map() + |> fixed_map() end # The idea behind a "sub" dtype is that in the dtype tree, you can replace