feat[next]: Add memory and disk-based caching to more workflow steps #1690

SF-N · 2024-10-15T15:26:59Z

Add memory and disk-based caching to other workflow steps and, therefore, removing unnecessary overhead of Program calls and significantly improving time to first computed value.

Changes:

setting cached = True for func_to_past_factory
wrapping the GTFN code generation into a CachedStep (using Diskcache) which is activated when setting otf_workflow__cached_translation=True, similar as in PR#1474 (without CachedStep)
Fixing hash function of ProgramDefinition
New tests for added functionality

This leads to a runtime decrease of about 25% for PMAP-G in the advect-uniform testcase (5 hours) after caches are populated.

TODOs:

improving hash functions of fingerprint_stage

Merge main

tests/next_tests/integration_tests/feature_tests/ffront_tests/test_execution.py

havogt · 2024-11-04T12:05:08Z

src/gt4py/next/config.py

@@ -73,6 +73,9 @@ def env_flag_to_bool(name: str, default: bool) -> bool:
 )


+GTFN_SOURCE_CACHE_DIR: str = os.environ.get(f"{_PREFIX}_GTFN_SOURCE_CACHE_DIR", "gtfn_cache")


I would prefer to avoid exposing a config option that is special to a certain backend. Is it necessary?

We introduced this since we wanted to avoid hardcoding it. In almost all cases, this should not change but e.g. in the tests, where the cache should be deleted afterwards, specifying a different directory would make sense.
Do you have a suggestion on how to make this configurable which you would prefer?

DaCe has also a cache directory, maybe we could use a common config option for compiled backends?

Yes, that sounds good to me. Do you agree @havogt?
And do you have a name in mind?

I originally proposed this, but after thinking about it again, and together with my other suggestion about how to deal with this in tests, I think it's not needed at least for this PR.

Ok, I removed it and hardcoded it in gtfn.py.

src/gt4py/next/ffront/stages.py

src/gt4py/next/program_processors/runners/gtfn.py

egparedes

Final comments.

src/gt4py/next/program_processors/runners/gtfn.py

.../next_tests/unit_tests/program_processor_tests/codegens_tests/gtfn_tests/test_gtfn_module.py

src/gt4py/next/ffront/stages.py

.../next_tests/unit_tests/program_processor_tests/codegens_tests/gtfn_tests/test_gtfn_module.py

egparedes · 2024-11-05T11:38:05Z

src/gt4py/next/config.py

@@ -73,6 +73,9 @@ def env_flag_to_bool(name: str, default: bool) -> bool:
 )


+GTFN_SOURCE_CACHE_DIR: str = os.environ.get(f"{_PREFIX}_GTFN_SOURCE_CACHE_DIR", "gtfn_cache")


I originally proposed this, but after thinking about it again, and together with my other suggestion about how to deal with this in tests, I think it's not needed at least for this PR.

.../next_tests/unit_tests/program_processor_tests/codegens_tests/gtfn_tests/test_gtfn_module.py

SF-N · 2024-11-05T12:27:53Z

src/gt4py/next/ffront/stages.py

@@ -148,4 +153,3 @@ def add_foast_located_node_to_fingerprint(
 ) -> None:
    add_content_to_fingerprint(obj.location, hasher)
    add_content_to_fingerprint(str(obj), hasher)
-    add_content_to_fingerprint(str(obj), hasher)


I am not sure, why this was done twice, does removing it make sense @egparedes / @DropD ?

It looks like a bug to me and I agree with your change.

egparedes · 2024-11-05T17:25:01Z

cscs-ci run

egparedes

I pushed the updated requirements and it looks ready to me. Let's just wait for @DropD lightweight review and we are good to merge from my point of view.

tests/next_tests/integration_tests/feature_tests/ffront_tests/test_execution.py

.../next_tests/unit_tests/program_processor_tests/codegens_tests/gtfn_tests/test_gtfn_module.py

SF-N · 2024-11-07T13:31:49Z

Thanks for highlighting the unused imports, I removed all of them. Applying ruff to the tests as well would help to avoid this in the future.

#1690 included a change to make the hash of an `itir.FencilDefinition` stable across multiple runs. This PR adopts the same change to an `itir.Program`,

tehrengruber and others added 30 commits February 28, 2024 10:31

Fix lowering_utils._expr_hash stability across runs

7e78704

Pass manager caching

7515d07

Fix format

54bbea0

Merge branch 'fix_caching2' into pass_manager_caching

0cfc583

Use content_hash

04d8459

Fix import

eac113c

Small fix

679521a

Small fix

2e3f17b

Merge branch 'fix_caching2' into pass_manager_caching

2803f41

Small fix

d6adb0e

Small cleanup

b39b22f

Merge branch 'fix_caching2' into pass_manager_caching

06c0384

Fix cache dir creation

bbc3b4d

Merge origin/main

3e0b021

Add todo

8f5aedd

Fix typo

e039057

Cleanup

5a8b525

Add tests

cf213c4

Merge pull request #1 from GridTools/main

b724a8d

Merge main

Merge remote-tracking branch 'origin/main'

6bd5652

Merge remote-tracking branch 'origin/main'

ff055f1

Extend lap_test to also run benchmark

cdfd6b5

Add pytest-benchmark to requirements-dev

6806f01

Disable pytest-xdist to enable benchmarking

fb93b73

remove -n from tox.ini

ad76088

Enable pytest-xdist and run benchmarks separately

256d9de

Change order for faster CI results

f00c60a

Add pytest flags and reformat

ccc35d0

Use larger grid for lap_benchmark

780a955

Add pytest.ini for benchmark comparison

3effb43

SF-N requested a review from egparedes November 4, 2024 11:59

havogt reviewed Nov 4, 2024

View reviewed changes

tests/next_tests/integration_tests/feature_tests/ffront_tests/test_execution.py Outdated Show resolved Hide resolved

Merge branch 'main' into optimize_program

e908009

havogt reviewed Nov 4, 2024

View reviewed changes

SF-N added 2 commits November 4, 2024 13:08

Merge branch 'main' into optimize_program

98ab673

Merge branch 'main' into optimize_program

8f15ee2

egparedes reviewed Nov 4, 2024

View reviewed changes

src/gt4py/next/ffront/stages.py Show resolved Hide resolved

egparedes reviewed Nov 4, 2024

View reviewed changes

src/gt4py/next/program_processors/runners/gtfn.py Show resolved Hide resolved

egparedes reviewed Nov 4, 2024

View reviewed changes

src/gt4py/next/program_processors/runners/gtfn.py Outdated Show resolved Hide resolved

Add docstrings and move tests

b7241bf

egparedes requested changes Nov 5, 2024

View reviewed changes

SF-N added 2 commits November 5, 2024 13:06

Double-check add_content_to_fingerprint

c7bcc6a

Merge branch 'main' into optimize_program

3f76e88

SF-N commented Nov 5, 2024

View reviewed changes

SF-N and others added 2 commits November 5, 2024 13:52

Address review comments

87d4e94

Update requirements

23a1432

egparedes approved these changes Nov 5, 2024

View reviewed changes

egparedes requested a review from DropD November 5, 2024 17:26

egparedes changed the title ~~feat[next]: Remove unnecessary overhead of Program calls~~ feat[next]: Add memory and disk-based caching to more workflow steps Nov 6, 2024

DropD requested changes Nov 7, 2024

View reviewed changes

SF-N added 2 commits November 7, 2024 12:14

Merge branch 'main' into optimize_program

fd6bde3

Address review comments

9404b4a

DropD approved these changes Nov 7, 2024

View reviewed changes

SF-N merged commit 1b9eb5c into GridTools:main Nov 7, 2024
51 checks passed

tehrengruber mentioned this pull request Nov 12, 2024

bug[next]: Fix ITIR program hash stability #1733

Merged

tehrengruber added a commit that referenced this pull request Nov 14, 2024

bug[next]: Fix ITIR program hash stability (#1733)

89fea8f

#1690 included a change to make the hash of an `itir.FencilDefinition` stable across multiple runs. This PR adopts the same change to an `itir.Program`,

This was referenced Nov 14, 2024

feature[next]: ITIR transformation & gtfn codegen caching #1474

Closed

next: GTFN Caching bug #1739

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat[next]: Add memory and disk-based caching to more workflow steps #1690

feat[next]: Add memory and disk-based caching to more workflow steps #1690

SF-N commented Oct 15, 2024 •

edited by egparedes

Loading

havogt Nov 4, 2024

SF-N Nov 4, 2024

edopao Nov 4, 2024

SF-N Nov 5, 2024

egparedes Nov 5, 2024

SF-N Nov 5, 2024

egparedes left a comment

egparedes Nov 5, 2024

SF-N Nov 5, 2024 •

edited

Loading

egparedes Nov 5, 2024

egparedes commented Nov 5, 2024

egparedes left a comment •

edited

Loading

SF-N commented Nov 7, 2024

		@@ -73,6 +73,9 @@ def env_flag_to_bool(name: str, default: bool) -> bool:
		)


		GTFN_SOURCE_CACHE_DIR: str = os.environ.get(f"{_PREFIX}_GTFN_SOURCE_CACHE_DIR", "gtfn_cache")

feat[next]: Add memory and disk-based caching to more workflow steps #1690

feat[next]: Add memory and disk-based caching to more workflow steps #1690

Conversation

SF-N commented Oct 15, 2024 • edited by egparedes 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

Choose a reason for hiding this comment

egparedes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SF-N Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egparedes commented Nov 5, 2024

egparedes left a comment • edited Loading

Choose a reason for hiding this comment

SF-N commented Nov 7, 2024

SF-N commented Oct 15, 2024 •

edited by egparedes

Loading

SF-N Nov 5, 2024 •

edited

Loading

egparedes left a comment •

edited

Loading