chore(backward): integrate backward compat data #2493

nsarlin-zama · 2025-07-01T09:40:22Z

closes: please link all relevant issues

PR content/description

Integrate backward compat data inside TFHE-rs main repo. This should simplify the process to add new data. Once this pr is merged, the old repo should be archived but kept to be able to run tests on old versions.

The lfs data will not be cloned by default to avoid impacting dev experience, but running make clone_backward_compat_data will clone them.

The data cache in ci is now based on the hash of the lfs files, because we previously used the commit sha of the target repo

This change is

nsarlin-zama · 2025-07-03T10:10:30Z

Note that cache seems to be temporary down?
I get "Failed to save: reserveCache failed: Cache service responded with 503" but it seems to also be the case on regular PR: https://github.com/zama-ai/tfhe-rs/actions/runs/16045513937/job/45275855427?pr=2514#step:11:25

IceTDrinker · 2025-07-03T11:07:35Z

Note that cache seems to be temporary down? I get "Failed to save: reserveCache failed: Cache service responded with 503" but it seems to also be the case on regular PR: https://github.com/zama-ai/tfhe-rs/actions/runs/16045513937/job/45275855427?pr=2514#step:11:25

I don't know if it's a problem on our end or with the github cache service 🤔

mayeul-zama

Looks good to me!
But I haven't reviewed the github action file change

I wonder if it would make sense to merge tfhe-backward-compat and backward-compatibility crates (in a later PR)

Thanks a lot for this PR, will be useful!

mayeul-zama · 2025-07-03T14:45:49Z

tests/backward_compatibility_tests.rs

-//! Tests breaking change in serialized data by trying to load historical data stored in https://github.com/zama-ai/tfhe-backward-compat-data.
-//! For each tfhe-rs module, there is a folder with some serialized messages and a [ron](https://github.com/ron-rs/ron)
+//! Tests breaking change in serialized data by trying to load historical data stored in
+//! `utils/tfhe-backward-compat-data`. For each tfhe-rs module, there is a folder with some serialized messages and a [ron](https://github.com/ron-rs/ron)


Could clarify that data is stored with git lfs

mayeul-zama · 2025-07-03T14:54:14Z

scripts/clone_backward_compat_data.sh

@@ -2,9 +2,9 @@



This file could be renamed pull_...

mayeul-zama · 2025-07-03T14:54:59Z

scripts/clone_backward_compat_data.sh

-else
-		git clone $1 -b $2 --depth 1 $3
-fi
+git lfs pull --include="$1/*" --exclude="*.hpu"


Are both include and exclude needed ?

I don't know I copied from hpu doc which uses an empty exclude

"Only paths which are matched by fetchinclude and not matched by fetchexclude will have objects fetched for them." from
https://manpages.ubuntu.com/manpages/focal/man1/git-lfs-pull.1.html

I guess we could remove exclude in every git lfs pull in the repo and only use the relevant include

Also, we could have a single pull_from_lfs script which takes a wildcard as argument
And 2 make targets pull_backward_compat_data and pull_hpu_files which call the same above script with different arguments

And call these targets instead of calling git lfs pull everywhere

what's the problem with having dedicated git lfs pull commands ?

This script checks that git lfs exits
IMO either we think it's useful and we should use this script everywhere
Or not and then we should remove it and call git lfs directly everywhere

@zama-ai/hardware would a command like make pull_hpu_files (or whatever name you want) be ok for you?

actually both include and exclude are needed because there is a default value for exclude in .lfsconfig so we need to override it

mayeul-zama · 2025-07-03T14:55:56Z

Makefile

@@ -263,6 +249,9 @@ install_mlc: install_rs_build_toolchain
 .PHONY: fmt # Format rust code
 fmt: install_rs_check_toolchain
 	cargo "$(CARGO_RS_CHECK_TOOLCHAIN)" fmt
+	cargo "$(CARGO_RS_CHECK_TOOLCHAIN)" -Z unstable-options -C utils/tfhe-backward-compat-data fmt


Could use BACKWARD_COMPAT_DATA_DIR here and other places (or remove this variable)

nsarlin-zama · 2025-07-04T08:18:27Z

I wonder if it would make sense to merge tfhe-backward-compat and backward-compatibility crates (in a later PR)

Technically there is no backward-compatibility crate, this is a module inside the tests crate for integration tests.
I think we might run into dependency issues if we merge them.

However I plan to split the backward-compat-data crate to simplify the multi-tfhe-dependency thing and add the possibility to generate only the data for a single version.

nsarlin-zama · 2025-07-04T09:17:46Z

I changed the heuristic to hint that the data may have not been pulled.
Before we checked if the dir existed but we cannot do that anymore. We suggest to clone the data if all the tests fail.

mayeul-zama · 2025-07-04T09:37:37Z

scripts/clone_backward_compat_data.sh

-else
-		git clone $1 -b $2 --depth 1 $3
-fi
+git lfs pull --include="$1/*" --exclude="*.hpu"


"Only paths which are matched by fetchinclude and not matched by fetchexclude will have objects fetched for them." from
https://manpages.ubuntu.com/manpages/focal/man1/git-lfs-pull.1.html

I guess we could remove exclude in every git lfs pull in the repo and only use the relevant include

Also, we could have a single pull_from_lfs script which takes a wildcard as argument
And 2 make targets pull_backward_compat_data and pull_hpu_files which call the same above script with different arguments

And call these targets instead of calling git lfs pull everywhere

nsarlin-zama · 2025-07-04T09:46:06Z

I guess but I don't want to touch hpu workflow in this pr

mayeul-zama · 2025-07-04T15:38:43Z

Makefile


-tests/$(BACKWARD_COMPAT_DATA_DIR): clone_backward_compat_data
+.PHONY: pull_hpu_files # Pull the hpu files
+pull_hpu_file:


Lacks an s

mayeul-zama · 2025-07-04T15:41:27Z

backends/tfhe-hpu-backend/README.md

@@ -217,7 +217,7 @@ source setup_hpu.sh --config v80
 > NB: Error that occurred when ".hpu" files weren't correctly fetch could be a bit enigmatic: `memory allocation of ... bytes failed`
 > If you encountered this issue, you should run the following command:
 > ```bash
-> git lfs pull --include="*" --exclude=""
+> make pull_hpu_files


This change could also be done in benchmark_hpu_integer.yml

mayeul-zama

Thanks a lot!

I'm not knowledgeable to review file .github/workflows/aws_tfhe_backward_compat_tests.yml

Reviewed 13 of 125 files at r1, 9 of 9 files at r3, 4 of 4 files at r4, 4 of 4 files at r6, all commit messages.
Reviewable status: 20 of 128 files reviewed, all discussions resolved (waiting on @IceTDrinker and @soonum)

IceTDrinker

questions/comments, looks good generally speaking

Reviewed 117 of 125 files at r1, 6 of 9 files at r3, 1 of 4 files at r4, 4 of 4 files at r6, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @soonum)

tests/backward_compatibility_tests.rs line 28 at r6 (raw file):

    };

    if !root_dir.exists() {

I would keep this if we move stuff around, we had an issue with Cargo manifest at some point ?

utils/tfhe-backward-compat-data/.linelint.yml line 1 at r6 (raw file):

ignore:

this file necessary ? feels like I saw the ignore for ron elsewhere

.github/workflows/aws_tfhe_backward_compat_tests.yml line 86 at r6 (raw file):

        with:
          path: |
            utils/tfhe-backward-compat-data/**/*.cbor

/data would be enough ? no need for globbing I think ?

.github/workflows/aws_tfhe_backward_compat_tests.yml line 105 at r6 (raw file):

        with:
          path: |
            utils/tfhe-backward-compat-data/**/*.cbor

same here

Makefile line 1055 at r6 (raw file):

.PHONY: test_backward_compatibility_ci
test_backward_compatibility_ci: install_rs_build_toolchain
	TFHE_BACKWARD_COMPAT_DATA_DIR="../$(BACKWARD_COMPAT_DATA_DIR)" RUSTFLAGS="$(RUSTFLAGS)" cargo $(CARGO_RS_BUILD_TOOLCHAIN) test --profile $(CARGO_PROFILE) \

../ seems a bit counterintuitive ?

Makefile line 1059 at r6 (raw file):

.PHONY: test_backward_compatibility # Same as test_backward_compatibility_ci but tries to clone the data repo first if needed
test_backward_compatibility: pull_backward_compat_data test_backward_compatibility_ci

what effect does the pull have if we are modifying lfs files to add new data e.g. ?

Code is taken from https://github.com/zama-ai/tfhe-backward-compat-data/tree/59a61798316b346e2ade6a1b45bac81c96c6fa67 Adapted to make ci work

nsarlin-zama

Reviewable status: 127 of 128 files reviewed, 5 unresolved discussions (waiting on @IceTDrinker, @mayeul-zama, and @soonum)

Makefile line 1055 at r6 (raw file):