test: Add trajectories in tests#691
Conversation
PierreQuinton
left a comment
There was a problem hiding this comment.
Should we check it with ruff and ty? Should we run it on the CI to check that there is no bugs? This last question also goes for profling and plots?
We do already. ruff has the default settings of what to check so it checks the whole repo, and ty is set in pyproject.toml to check src (except nashmtl) and tests.
Long term I would say yes for all three, or to at least try. But we need to make those scripts extremely fast (so that CI isn't slowed down), or at least make a parametrization of those scripts that is extremely fast, so it's quite a bit of work. Also if we can't manage to make this run fast, it's not worth IMO. |
This PR adds a trajectories package in TorchJD/tests so that we can run the trajectories directly within TorchJD. The goal is to archive the trajectories repo entirely.
I think this has the advantage of making the trajectories scripts much more visible to people who browse the TorchJD repo, and of making it more easily usable for us (for example, we'll be able to very quickly see the trajectories of upgrad with proxsuite if we merge this. Also, it makes it easier to maintain this repo (because ruff will warn us if something breaks, for example if we change the interface of autojac or of aggregators). So it should be really easy to maintain this.
Maybe one days we should have tests to ensure that the scripts such as this, the interactive plotter, the static plotter, the profiler, and so on, are not broken. But I think it's fine if they break, and we repair when we need them, so it's not really urgent. And type checking makes it unlikely to break very frequently.
I think that after merging this, we should make a few easy improvements: