build-jax.sh: update default source dir

[olupton]

This changed in #371.

[nouiz]

@DwarKapex Why this changed upstream?
And how the CI didn't crash with this issue?

[nouiz]

I'll merge now as the CI seems good and others are OOTO.
But it would be good to understand why the CI didn't crash and make sure it crashes if such issue appear again.

[nouiz]

I'll merge now as the CI seems good and others are OOTO. But it would be good to understand why the CI didn't crash and make sure it crashes if such issue appear again.

I found, Dockerfile.jax do:

RUN build-jax.sh \
    --bazel-cache ${BAZEL_CACHE} \
    --src-path-jax ${SRC_PATH_JAX} \
    --src-path-xla ${SRC_PATH_XLA} \
    --sm all \
    --xla-arm64-patch /opt/xla-arm64-neon.patch \
    --clean

we need to be able to overwrite the JAX, so we need to pass the flag in that case. So I'm not sure how to make it crash next time.

[olupton]

JAX-Toolbox/.github/container/Dockerfile.jax

Lines 53 to 59 in 5bba3d8

	RUN build-jax.sh \
	--bazel-cache ${BAZEL_CACHE} \
	--src-path-jax ${SRC_PATH_JAX} \
	--src-path-xla ${SRC_PATH_XLA} \
	--sm all \
	--xla-arm64-patch /opt/xla-arm64-neon.patch \
	--clean

the default isn't used in the Dockerfile.

I'm not sure if there's a reason the Dockerfile has to be parametrised on this.

[nouiz]

the default isn't used in the Dockerfile.

I see it on line 10 of the Dockerfile.jax:

JAX-Toolbox/.github/container/Dockerfile.jax

Line 10 in 5bba3d8

ARG SRC_PATH_JAX=/opt/jax

I'm not sure if there's a reason the Dockerfile has to be parametrised on this.

I think it is done like this to allows the manual start of CI workflow to specify which commit/branch/version we want to use.
We could use a special token, like DEFAULT, to use the default in the build-jax.sh script. This would prevent to duplicate the default value at 2 places. But this is a rare thing that we change this default, so I'm not sure it is worthwhile the extra complexity.

[olupton]

the default isn't used in the Dockerfile.

I see it on line 10 of the Dockerfile.jax:

JAX-Toolbox/.github/container/Dockerfile.jax

Line 10 in 5bba3d8

ARG SRC_PATH_JAX=/opt/jax

I meant that the default inside build-jax.sh is not used when it is called from the Dockerfile, because --src-jax-path is always passed.

I'm not sure if there's a reason the Dockerfile has to be parametrised on this.

I think it is done like this to allows the manual start of CI workflow to specify which commit/branch/version we want to use. We could use a special token, like DEFAULT, to use the default in the build-jax.sh script. This would prevent to duplicate the default value at 2 places. But this is a rare thing that we change this default, so I'm not sure it is worthwhile the extra complexity.

Agree we don't need more complexity. (Not sure why changing commit/branch/version would need a different source dir.)

[nouiz]

Agree we don't need more complexity. (Not sure why changing commit/branch/version would need a different source dir.)

You are right, my explanation wasn't the right one. In the dockerfile, it isn't useful. We should be able to remove it there.
This parameters should be only useful for dev that can want to change the directory.