tenstorrent / tt-metal

:metal: TT-NN operator library, and TT-Metalium low level kernel programming model.
Apache License 2.0
391 stars 47 forks source link

Replace checks of "CI" environment variable with conftest fixture in model tests #10402

Open skhorasganiTT opened 1 month ago

skhorasganiTT commented 1 month ago

All checks of os.getenv("CI") should be replaced with the is_ci_env fixture where possible, as per the request of @tt-rkim.

mtairum commented 1 month ago

@skhorasganiTT How important is this requirement?

We rely on Mixtral env flags to locate the weights and cache files. This needs to be exported before we load the model library, which pins the location during init.

Using fixtures for this scenario means either moving the library loading to inside the test call or passing these path flags when loading the model config.

tt-rkim commented 1 month ago

I would encourage model team to replace everything with this. We need to centralize our environment variable usage and also reduce our workload in the future when we move caching file systems away from MLPerf.

If you mean TtModelArgs folder values, could you perhaps initialize these when TtModelArgs is constructed in mistral7b?

mtairum commented 1 month ago

Done for Mistral and Mixtral.

tt-rkim commented 1 month ago

Thank you!!!

@kevinmiTT11 @cglagovichTT any update on llama?

cglagovichTT commented 1 month ago

This is on the backlog - we should be able to get to it within a week