Skip to content
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

MàJ versions Pytorch et Pytorch-Geometric #105

Merged
merged 31 commits into from
Feb 6, 2024
Merged

Conversation

CharlesGaydon
Copy link
Collaborator

@CharlesGaydon CharlesGaydon commented Jan 2, 2024

Checks

  • Pytest
  • Vérifier rétrocompatibilité avec précédent modèles
  • Versionner
  • Vérifier résultats d'inférence
  • FIx de la version de urllib3 en CICD pour Comet (qui ne passait pas dans Fix dependencies #95)
  • Eval du gain en temps en inférence (dalle 888000_6614000) : avant MàJ 80sec d'inférence (total: 135sec), après MàJ : 82sec d'inférence (total: 131sec) -> pas de différence notable dans ce cas précis.
  • Eval du gain en temps en apprentissage
  • Vérification des implications de --> pour une prochaine PR

You are using a CUDA device ('NVIDIA A40') that has Tensor Cores. To properly utilize them, you should set torch.set_float32_matmul_precision('medium' | 'high') which will trade-off precision for performance. For more details, read https://pytorch.org/docs/stable/generated/torch.set_float32_matmul_precision.html#torch.set_float32_matmul_precision

Versions de l'env installé:
image

@CharlesGaydon
Copy link
Collaborator Author

@MichelDaab @leavauchier Pour info, j'ai fait cette petite PR de mise à jour. Les versions n'avaient pas été updatées depuis deux ans au moins. Les tests passent en local, mais on a toujours des soucis avec la CICD - je soupconne à nouveau un problème de taille de la VM. Le problème est arrivé en changeant la version pour urllib3, mais c'est peut-être une coïncidence : changer les requirements oblige à reconstruire une nouvelle image, et boom la mémoire sature.

Ca me semblerait bien qu'avant d'aller plus loin on trouve le problème... Histoire que les routines de déploiement soient opérationnelles quand on en a besoin.

@MichelDaab
Copy link
Collaborator

Es-tu sûr que le problème vient des requirements /nouvelle image / taille mémoire ? Pour faire la modif de Léa sur l'EPSG en décembre il me semble que j'ai dû faire refaire une image aussi (j'ai souvenir que le CICD a tourné pendant un long moment une fois), et il n'y a pas eu de problème inhabituel

@CharlesGaydon
Copy link
Collaborator Author

Es-tu sûr que le problème vient des requirements /nouvelle image / taille mémoire ? Pour faire la modif de Léa sur l'EPSG en décembre il me semble que j'ai dû faire refaire une image aussi (j'ai souvenir que le CICD a tourné pendant un long moment une fois), et il n'y a pas eu de problème inhabituel

Alors ce que je sais c'est que c'est un problème de mémoire insuffisante au moment de la création de l'environnement dans l'image docker.

Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... Killed
The command '/bin/sh -c curl -sLo /miniconda.sh https://repo.continuum.io/miniconda/Miniconda3-py39_4.10.3-Linux-x86_64.sh && chmod +x /miniconda.sh && /miniconda.sh -b -p /miniconda && rm /miniconda.sh && /miniconda/bin/conda env update -n base -f /app/environment.yml && rm /app/environment.yml && /miniconda/bin/conda clean -ya' returned a non-zero code: 137

Ce code c'est un code de mémoire excedée.
Je lis ici qu'il est possible d'ajouter de la mémoire SWAP. Par défaut docker peut utiliser toutes les ressources du système, donc utilise déjà tout le swap possible.

@CharlesGaydon
Copy link
Collaborator Author

CharlesGaydon commented Jan 3, 2024

Il n'y a pas de swap actuellement

sudo swapon --show
NAME TYPE SIZE USED PRIO
/dev/dm-1 partition 3,7G 487M -2

Mais on a un peu de marge pour ajouter un swap (7.4Go de dispo)

df -h /
Sys. de fichiers Taille Utilisé Dispo Uti% Monté sur
/dev/mapper/Vol1-systeme 9,1G 7,4G 1,3G 86% /

Ce que je ne saisis pas c'est que les images docker sur la machine semblent sommer à plus que ce qui est indiqué cmme utilisé ici

sudo docker system df
TYPE TOTAL ACTIVE SIZE RECLAIMABLE
Images 10 0 22.09GB 22.09GB (100%)
Containers 0 0 0B 0B
Local Volumes 1 0 0B 0B
Build Cache 0 0 0B 0B

@MichelDaab
Copy link
Collaborator

Peut-être que docker est sur /data, un autre disque que /dev ?

@CharlesGaydon
Copy link
Collaborator Author

Peut-être que docker est sur /data, un autre disque que /dev ?

Ah oui probablement !

/dev/mapper/Vol1-data 295G 33G 247G 12% /var/data

@CharlesGaydon
Copy link
Collaborator Author

CharlesGaydon commented Jan 8, 2024

Comparaison des temps d'apprentissage, sur 100 epochs, avec 2x3 GPUs:
image
--> Réduction de 30% du temps d'apprentissage !
--> Val loss plus faible mais diminution des IoU de validation (rouge)
image
--> Pourtant, performances du modèle final conservées. Problème de logs ?

XP Pytorch 1.11 Pytorch 2.1
Convergence epoch 37 epoch 44
Val loss 0.106 0.0100
Test IoU 0.690 0.696

Pytorch 1.11
image
Pytorch 2.1
image

@CharlesGaydon
Copy link
Collaborator Author

CharlesGaydon commented Jan 8, 2024

La différence dans les métriques d'IoY à l'apprentissage s'explique par la MàJ de torchmetrics qui change le comportement de compute(...) 👍🏻

From v1.2 onward compute() will no longer automatically call reset(), and it is up to the user to reset metrics between epochs, except in the case where the metric is directly passed to LightningModule's self.log.

Je vais donc reset les métriques manuellement, et vérifier qu'en poursuivant l'apprentissage on se retrouve avec les val IoU attendues.
Par ailleurs, le log de experiment_logs_dirpath semble être désactivé, je vais voir ce qu'il en est.

@CharlesGaydon
Copy link
Collaborator Author

Correctif des métrique validé : en reprenant l'entraînement après correctif (violet) les métriques sont équivalentes à celles obtenues avec la branche main (bleu)
image

environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
package_metadata.yaml Show resolved Hide resolved
@CharlesGaydon
Copy link
Collaborator Author

J'ai fait quelques tests de compilation de modèle (on peut encore gagner en performances, avec un facteur 1.5 à 2.5 !).
Je rencontre un soucis, j'ai ouvert une issue sur torch_geometric. Pas obligé d'ajouter cette fonctionnalité dans cette PR, on pourra séparer.

environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
myria3d/metrics/iou.py Outdated Show resolved Hide resolved
myria3d/metrics/iou.py Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
environment.yml Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
environment.yml Show resolved Hide resolved
myria3d/metrics/iou.py Show resolved Hide resolved
@@ -217,11 +216,13 @@ def _run_test_right_after_training(
tmp_paths_overrides = _make_list_of_necesary_hydra_overrides_with_tmp_paths(
toy_dataset_hdf5_path, tmpdir
)
devices = "[0]" if accelerator == "gpu" else 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why enforcing it now? it looks like the previous versions could let you choose your gpu(s)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In previous versions GPU 0 was always used. This is just a small refactor due to a change of signature of trainer, which requires two params: accelerator and devices, and deprecates gpus.
I made the choice to always use the same gpu to be able to run tests locally with the possibility to willingly avoid conflict with other processes (on other gpus, not on gpu 0). But I just discovered that lightning can do that automatically:
https://lightning.ai/docs/pytorch/stable/accelerators/gpu_basic.html#find-usable-cuda-devices
So i'll update the test.

@CharlesGaydon
Copy link
Collaborator Author

I made the requested changes @leavauchier

Copy link
Collaborator

@leavauchier leavauchier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants