-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Improving step by step instructions for structured_parser #985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
57fa72e
Improving step by step instructions, requirements and variable initia…
varunfb 78701d5
Fixing spell checker
varunfb 253d85e
Fixing spell checker
varunfb 9bba286
Replaced non-existant dataset
varunfb c50ddbf
Fixing failing github runner
varunfb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,5 +72,4 @@ jobs: | |
| with: | ||
| paths: | | ||
| **/*.xml | ||
| !**/AndroidManifest.xml | ||
| if: always() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -196,6 +196,7 @@ def _run_inference( | |||||
| artifact_types = [r[0] for r in requests] | ||||||
| inference_requests = [r[1] for r in requests] | ||||||
|
|
||||||
| response_batch = [] | ||||||
| if backend == "offline-vllm": | ||||||
| request_batch = InferenceUtils.make_vllm_batch(inference_requests) | ||||||
| response_batch = InferenceUtils.run_vllm_inference(request_batch) | ||||||
|
|
@@ -304,79 +305,6 @@ def from_pdf(pdf_path: str, artifact_types: List[str]) -> List[ExtractedPage]: | |||||
|
|
||||||
| return pdf_pages | ||||||
|
|
||||||
| # @staticmethod | ||||||
| # async def _run_inference_async( | ||||||
| # requests: List[Tuple[str, InferenceRequest]], | ||||||
| # ) -> List[Tuple[str, str]]: | ||||||
| # """ | ||||||
| # Run inference asynchronously for all requests. | ||||||
|
|
||||||
| # Args: | ||||||
| # requests: List of tuples containing (artifact_type, inference_request) | ||||||
|
|
||||||
| # Returns: | ||||||
| # List of tuples containing (artifact_type, response) | ||||||
|
|
||||||
| # Raises: | ||||||
| # ValueError: If the backend is not supported | ||||||
| # """ | ||||||
| # backend = config["model"].get("backend") | ||||||
| # if backend not in SUPPORTED_BACKENDS: | ||||||
| # raise ValueError( | ||||||
| # f"Allowed config.model.backend: {SUPPORTED_BACKENDS}, got unknown value: {backend}" | ||||||
| # ) | ||||||
|
|
||||||
| # artifact_types = [r[0] for r in requests] | ||||||
| # inference_requests = [r[1] for r in requests] | ||||||
|
|
||||||
| # if backend == "offline-vllm": | ||||||
| # request_batch = InferenceUtils.make_vllm_batch(inference_requests) | ||||||
| # response_batch = InferenceUtils.run_vllm_inference(request_batch) | ||||||
| # elif backend == "openai-compat": | ||||||
| # tasks = [ | ||||||
| # InferenceUtils.async_run_openai_inference(request) | ||||||
| # for request in inference_requests | ||||||
| # ] | ||||||
| # response_batch = await asyncio.gather(*tasks) | ||||||
|
|
||||||
| # return list(zip(artifact_types, response_batch)) | ||||||
|
|
||||||
| # @staticmethod | ||||||
| # async def from_image_async( | ||||||
| # img_path: str, | ||||||
| # artifact_types: Union[List[str], str], | ||||||
| # ) -> ArtifactCollection: | ||||||
| # """ | ||||||
| # Extract artifacts from an image asynchronously. | ||||||
|
|
||||||
| # Args: | ||||||
| # img_path: Path to the image file | ||||||
| # artifact_types: Type(s) of artifacts to extract | ||||||
|
|
||||||
| # Returns: | ||||||
| # ArtifactCollection: Extracted artifacts | ||||||
|
|
||||||
| # Raises: | ||||||
| # ValueError: If the backend is not supported | ||||||
| # FileNotFoundError: If the image file doesn't exist | ||||||
| # """ | ||||||
| # if not os.path.exists(img_path): | ||||||
| # raise FileNotFoundError(f"Image file not found: {img_path}") | ||||||
|
|
||||||
| # if isinstance(artifact_types, str): | ||||||
| # artifact_types = [artifact_types] | ||||||
|
|
||||||
| # # Prepare inference requests | ||||||
| # requests = ArtifactExtractor._prepare_inference_requests( | ||||||
| # img_path, artifact_types | ||||||
| # ) | ||||||
|
|
||||||
| # # Run inference asynchronously | ||||||
| # responses = await ArtifactExtractor._run_inference_async(requests) | ||||||
|
|
||||||
| # # Process responses | ||||||
| # return ArtifactExtractor._process_responses(responses) | ||||||
|
|
||||||
|
|
||||||
| def get_artifact_types(text: bool, tables: bool, images: bool) -> List[str]: | ||||||
| """ | ||||||
|
|
@@ -422,16 +350,16 @@ def get_target_files(target_path: str) -> List[Path]: | |||||
| if not os.path.exists(target_path): | ||||||
| raise FileNotFoundError(f"Target path not found: {target_path}") | ||||||
|
|
||||||
| target_path = Path(target_path) | ||||||
| if target_path.is_file() and target_path.suffix not in SUPPORTED_FILE_TYPES: | ||||||
| target_path_obj = Path(target_path) | ||||||
| if target_path_obj.is_file() and target_path_obj.suffix not in SUPPORTED_FILE_TYPES: | ||||||
| raise ValueError( | ||||||
| f"Unsupported file type: {target_path.suffix}. Supported types: {SUPPORTED_FILE_TYPES}" | ||||||
| f"Unsupported file type: {target_path_obj.suffix}. Supported types: {SUPPORTED_FILE_TYPES}" | ||||||
| ) | ||||||
|
|
||||||
| targets = ( | ||||||
| [target_path] | ||||||
| if target_path.is_file() | ||||||
| else [f for f in target_path.iterdir() if f.suffix in SUPPORTED_FILE_TYPES] | ||||||
| [target_path_obj] | ||||||
| if target_path_obj.is_file() | ||||||
| else [f for f in target_path_obj.iterdir() if f.suffix in SUPPORTED_FILE_TYPES] | ||||||
| ) | ||||||
| logger.debug(f"Processing {len(targets)} files") | ||||||
| if not targets: | ||||||
|
|
@@ -456,7 +384,7 @@ def process_files( | |||||
| out_json = [] | ||||||
| for target in targets: | ||||||
| try: | ||||||
| artifacts = ArtifactExtractor.from_pdf(target, artifact_types) | ||||||
| artifacts = ArtifactExtractor.from_pdf(str(target), artifact_types) | ||||||
| out_json.extend(artifacts) | ||||||
| except Exception as e: | ||||||
| logger.error(f"Failed to process {target}: {e}") | ||||||
|
|
@@ -485,6 +413,7 @@ def save_results( | |||||
| output_dir.mkdir(parents=True, exist_ok=True) | ||||||
|
|
||||||
| # Save to JSON file | ||||||
| output_path = None | ||||||
| try: | ||||||
| output_path = output_dir / f"artifacts_{timestamp}.json" | ||||||
| json_content = json.dumps(data, indent=2) | ||||||
|
|
@@ -562,8 +491,8 @@ def main( | |||||
| results = process_files(targets, artifact_types) | ||||||
|
|
||||||
| # Save results | ||||||
| target_path = Path(target_path) | ||||||
| output_dir = target_path.parent / "extracted" | ||||||
| target_path_obj = Path(target_path) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| output_dir = target_path_obj.parent / "extracted" | ||||||
| save_results( | ||||||
| output_dir, | ||||||
| results, | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,32 +1,36 @@ | ||
| # Copyright (c) Meta Platforms, Inc. and affiliates. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change doesn't relate to the recipe, maybe you could remove it from this PR? |
||
| # This software may be used and distributed according to the terms of the Llama 2 Community License Agreement. | ||
|
|
||
| import pytest | ||
| from dataclasses import dataclass | ||
| from functools import partial | ||
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
| from datasets import load_dataset | ||
|
|
||
|
|
||
| @dataclass | ||
| class Config: | ||
| model_type: str = "llama" | ||
|
|
||
|
|
||
| try: | ||
| load_dataset("Samsung/samsum") | ||
| load_dataset("knkarthick/samsum") | ||
| SAMSUM_UNAVAILABLE = False | ||
| except ValueError: | ||
| SAMSUM_UNAVAILABLE = True | ||
|
|
||
|
|
||
| @pytest.mark.skipif(SAMSUM_UNAVAILABLE, reason="Samsum dataset is unavailable") | ||
| @pytest.mark.skip_missing_tokenizer | ||
| @patch('llama_cookbook.finetuning.train') | ||
| @patch('llama_cookbook.finetuning.AutoTokenizer') | ||
| @patch("llama_cookbook.finetuning.train") | ||
| @patch("llama_cookbook.finetuning.AutoTokenizer") | ||
| @patch("llama_cookbook.finetuning.AutoConfig.from_pretrained") | ||
| @patch("llama_cookbook.finetuning.AutoProcessor") | ||
| @patch("llama_cookbook.finetuning.MllamaForConditionalGeneration.from_pretrained") | ||
| @patch('llama_cookbook.finetuning.LlamaForCausalLM.from_pretrained') | ||
| @patch('llama_cookbook.finetuning.optim.AdamW') | ||
| @patch('llama_cookbook.finetuning.StepLR') | ||
| @patch("llama_cookbook.finetuning.LlamaForCausalLM.from_pretrained") | ||
| @patch("llama_cookbook.finetuning.optim.AdamW") | ||
| @patch("llama_cookbook.finetuning.StepLR") | ||
| def test_samsum_dataset( | ||
| step_lr, | ||
| optimizer, | ||
|
|
@@ -39,11 +43,13 @@ def test_samsum_dataset( | |
| mocker, | ||
| setup_tokenizer, | ||
| llama_version, | ||
| ): | ||
| ): | ||
| from llama_cookbook.finetuning import main | ||
|
|
||
| setup_tokenizer(tokenizer) | ||
| get_model.return_value.get_input_embeddings.return_value.weight.shape = [32000 if "Llama-2" in llama_version else 128256] | ||
| get_model.return_value.get_input_embeddings.return_value.weight.shape = [ | ||
| 32000 if "Llama-2" in llama_version else 128256 | ||
| ] | ||
| get_mmodel.return_value.get_input_embeddings.return_value.weight.shape = [0] | ||
| get_config.return_value = Config() | ||
|
|
||
|
|
@@ -55,7 +61,7 @@ def test_samsum_dataset( | |
| "use_peft": False, | ||
| "dataset": "samsum_dataset", | ||
| "batching_strategy": "padding", | ||
| } | ||
| } | ||
|
|
||
| main(**kwargs) | ||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this change. Currently, images is quite overloaded as we're using it for pictures as well as infographics. What are your thoughts on extracting charts either as a a) "table" artifact, or b) a separate artifact of its own?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline, lets refactor this to --charts as next steps.