make demos_pool a local var rather than a separate stream #1436

dafnapension · 2024-12-13T14:51:14Z

Ensures pool of demo instances is made of distinct items, and suffices for each ordinary instance, even if that ordinary instance is identical to one of the demo instances in the pool (see issue #1416).
Demo instances do not constitute a stream, thereby do not get loaded with demo instances by themselves (see issue #1210)

Extended functionality: Allow an input of a demos_pool, or an input in which (some or all) instances are already loaded each with demos

elronbandel · 2024-12-18T08:47:23Z

src/unitxt/standard.py

+                if item["input_fields"] == instance["input_fields"]:
+                    not_selected_from_from_stream.append(instance)
+                    break
+            demos_pool.append(instance)


Suggested change

demos_pool.append(instance)

demos_pool.append(instance)

demos_pool_set.add(str(insance["input_fields"]))

elronbandel · 2024-12-18T08:50:29Z

src/unitxt/standard.py

+
+        if len(demos_pool) < self.demos_pool_size:
+            raise ValueError(
+                f"Unable to fetch enough ({self.demos_pool_size}) instances from stream {self.from_stream} for the demos_pool"


Suggested change

f"Unable to fetch enough ({self.demos_pool_size}) instances from stream {self.from_stream} for the demos_pool"

f"Unable to fetch enough ({self.demos_pool_size}) instances from stream {self.from_stream} for the demos_pool, condsider increasing loader_limit or less strict stream filtering"

elronbandel · 2024-12-18T08:55:24Z

src/unitxt/standard.py

@@ -28,8 +40,72 @@


 # Used to give meaningful name to recipe steps
-class CreateDemosPool(SeparateSplit):
-    pass
+class CreateDemosPoolAndSpreadDemos(MultiStreamOperator):


Suggested change

class CreateDemosPoolAndSpreadDemos(MultiStreamOperator):

class CreateDemosPoolAndAssignDemosToInstaces(MultiStreamOperator):

elronbandel · 2024-12-18T08:56:34Z

src/unitxt/standard.py

+
+        if self.sample is None:
+            return MultiStream.from_generators(new_streams)
+        self.sample.demos_pool = demos_pool


Suggested change

self.sample.demos_pool = demos_pool

self.sample.set_demos_pool(demos_pool)

elronbandel · 2024-12-18T08:58:55Z

src/unitxt/splitters.py

-class Sample(InstanceOperatorWithMultiStreamAccess):
-    from_stream: str
+class Sample(InstanceOperator):
+    demos_pool: List[Dict[str, Any]] = None


Suggested change

demos_pool: List[Dict[str, Any]] = None

elronbandel · 2024-12-18T09:03:27Z

src/unitxt/splitters.py

@@ -350,30 +350,21 @@ def get_sample_size(self, instance) -> int:
        pass

    def process(


Suggested change

def process(

@property

def is_demos_pool_set(self, instance):

return "demos_pool" in instance and instance["demos_pool"] is not None and isinstance(instance["demos_pool"], list) and len(instance["demos_pool"]) > self.get_sample_size(instance)

def process(

elronbandel

Split operators to to CreateDemosPool and AssignDemosToInstances
Allow having pre made "demos_pool" or "demos" in instances
Check that every instance contain only pointer to the demos pool and no copy of it (god forbid)

elronbandel · 2024-12-22T08:15:24Z

src/unitxt/standard.py

+    from_stream: str = None
+    demos_pool_size: int = None
+    remove_targets_from_source_split: bool = None
+    to_field: str = "_demos_pool_"


Suggested change

to_field: str = "_demos_pool_"

to_field: str = constants.demos_pool_field

elronbandel · 2024-12-22T08:18:11Z

src/unitxt/standard.py

@@ -59,14 +239,18 @@ class BaseRecipe(Recipe, SourceSequentialOperator):
    test_refiner: StreamRefiner = OptionalField(default_factory=StreamRefiner)

    demos_pool_size: int = None
+    given_demos_pool: List[Dict[str, Any]] = None


Suggested change

given_demos_pool: List[Dict[str, Any]] = None

demos_pool: List[Dict[str, Any]] = None

elronbandel · 2024-12-22T08:22:12Z

src/unitxt/standard.py


 constants = get_constants()
 settings = get_settings()
 logger = get_logger()


 # Used to give meaningful name to recipe steps
-class CreateDemosPool(SeparateSplit):
-    pass
+class CreateDemosPool(MultiStreamOperator):


Suggested change

class CreateDemosPool(MultiStreamOperator):

class AddDemosPool(Set):

field = constants.demos_pool_field

class CreateDemosPool(MultiStreamOperator):

elronbandel · 2024-12-22T08:29:05Z

tests/library/test_recipe.py

+        recipe2 = DatasetRecipe(
+            card="cards.wnli",
+            template_card_index=0,
+            given_demos_pool=for_demos,


Suggested change

given_demos_pool=for_demos,

given_demos_pool=[

{"text": "hello world", "label": "pre"} # task complient dicts

],

…estart train repeatedly. Fix needed in Loaders. Allow a given demos_pool, or input stream-instances already loaded with demos Signed-off-by: dafnapension <[email protected]>

Signed-off-by: dafnapension <[email protected]>

dafnapension changed the title ~~update metadata just once in a recipe~~ make demos_pool a local var rather than a separate stream Dec 14, 2024

dafnapension force-pushed the demos_experimental branch from 781f19c to 850a7c0 Compare December 14, 2024 16:48

dafnapension mentioned this pull request Dec 14, 2024

Dataset generation error when num_demos is equal to demos_pool_size #1416

Open

dafnapension force-pushed the demos_experimental branch from a53f14e to 95a9560 Compare December 15, 2024 08:43

dafnapension requested a review from yoavkatz December 15, 2024 08:43

elronbandel reviewed Dec 18, 2024

View reviewed changes

elronbandel requested changes Dec 18, 2024

View reviewed changes

dafnapension force-pushed the demos_experimental branch 3 times, most recently from 03ffdde to ccebc83 Compare December 20, 2024 17:50

elronbandel reviewed Dec 22, 2024

View reviewed changes

dafnapension force-pushed the demos_experimental branch from 934c9a4 to 028a910 Compare December 22, 2024 09:45

dafnapension added 6 commits December 22, 2024 16:20

make demos_pool a variable, rather than a separate stream. Needs to r…

8d79f85

…estart train repeatedly. Fix needed in Loaders. Allow a given demos_pool, or input stream-instances already loaded with demos Signed-off-by: dafnapension <[email protected]>

renamed Sample->AssignDemosToInstance and emptied all but BaseRecipe

6a5c823

Signed-off-by: dafnapension <[email protected]>

StandardRecipe->DatasetRecipe and standard_recipe->dataset_recipe

7471087

Signed-off-by: dafnapension <[email protected]>

all recipes are DatasetRecipe

a97bb3e

Signed-off-by: dafnapension <[email protected]>

separate AddDemosPool from CreateDemosPool

2d9f3b2

Signed-off-by: dafnapension <[email protected]>

add deprecation for old recipes, and update docs

9d377d6

Signed-off-by: dafnapension <[email protected]>

dafnapension force-pushed the demos_experimental branch from 24342d7 to 9d377d6 Compare December 22, 2024 14:38

dafnapension added 3 commits December 22, 2024 16:44

fixed deprecation message

5aa89d2

Signed-off-by: dafnapension <[email protected]>

secrets

fe8ccb2

Signed-off-by: dafnapension <[email protected]>

secrets

3eb8eed

Signed-off-by: dafnapension <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make demos_pool a local var rather than a separate stream #1436

make demos_pool a local var rather than a separate stream #1436

dafnapension commented Dec 13, 2024 •

edited

Loading

elronbandel Dec 18, 2024

elronbandel Dec 18, 2024

elronbandel Dec 18, 2024

elronbandel Dec 18, 2024

elronbandel Dec 18, 2024

elronbandel Dec 18, 2024 •

edited

Loading

elronbandel left a comment

elronbandel Dec 22, 2024

elronbandel Dec 22, 2024

elronbandel Dec 22, 2024

elronbandel Dec 22, 2024

	demos_pool.append(instance)
	demos_pool.append(instance)
	demos_pool_set.add(str(insance["input_fields"]))

	f"Unable to fetch enough ({self.demos_pool_size}) instances from stream {self.from_stream} for the demos_pool"
	f"Unable to fetch enough ({self.demos_pool_size}) instances from stream {self.from_stream} for the demos_pool, condsider increasing loader_limit or less strict stream filtering"

	class CreateDemosPoolAndSpreadDemos(MultiStreamOperator):
	class CreateDemosPoolAndAssignDemosToInstaces(MultiStreamOperator):

	self.sample.demos_pool = demos_pool
	self.sample.set_demos_pool(demos_pool)

		@@ -350,30 +350,21 @@ def get_sample_size(self, instance) -> int:
		pass

		def process(

-    def process(
+    @property
+    def is_demos_pool_set(self, instance):
+          return "demos_pool" in instance and instance["demos_pool"] is not None and isinstance(instance["demos_pool"], list) and len(instance["demos_pool"]) > self.get_sample_size(instance)
+    def process(

	to_field: str = "_demos_pool_"
	to_field: str = constants.demos_pool_field

	given_demos_pool: List[Dict[str, Any]] = None
	demos_pool: List[Dict[str, Any]] = None

-            given_demos_pool=for_demos,
+            given_demos_pool=[
+            {"text": "hello world", "label": "pre"} # task complient dicts
+            ],

make demos_pool a local var rather than a separate stream #1436

Are you sure you want to change the base?

make demos_pool a local var rather than a separate stream #1436

Conversation

dafnapension commented Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elronbandel Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

elronbandel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dafnapension commented Dec 13, 2024 •

edited

Loading

elronbandel Dec 18, 2024 •

edited

Loading