Cambridge-ICCS / FTorch

A library for directly calling PyTorch ML models from Fortran.
https://cambridge-iccs.github.io/FTorch/
MIT License
48 stars 11 forks source link

Introduce integration tests based on the examples #118

Closed jwallwork23 closed 1 month ago

jwallwork23 commented 2 months ago

Closes #115.

Turns existing examples into integration tests using CTest.

jwallwork23 commented 2 months ago

That didn't turn out to be too difficult. A few notes:

  1. Couldn't figure out how to get the Python scripts under CTest without making them executable - hopefully not an issue.
  2. The 2nd example depends on numpy but this wasn't in the requirements so I added it.
  3. I used a Python venv approach because that's what I'm familiar with and know how to easily determine the ${Torch_DIR} env variable.
  4. Added a "proof of existence" assert at the end of the pt2ts.py scripts in the first two examples. Maybe this is something useful to have in the version in src, too?
  5. Note that I had to truncate the outputs to 6 d.p. in the 2nd example to get the tests to pass.
  6. Skipped the 3rd demo (multi-GPU) because we don't currently have access to GPUs in the CI.
dorchard commented 2 months ago

Can you add some documentation about how to run locally?

jatkinson1000 commented 2 months ago

FTorch development meeting discussion:

jwallwork23 commented 2 months ago

@jatkinson1000 @dorchard Okay, I think I addressed the requests above. If FTorch is built with -DCMAKE_BUILD_TESTS=TRUE then the examples get build as integration tests in src/test/examples. The tests can then be run by going to the subdirectory of interest and running ctest, as described in src/test/README.md.

Note that I considered a different approach 115_integration_tests_take2 that moves the CMakeLists.txt to the root level (as well as moving the recommended build directory down one level), rather than copying over example files. I think either could work, just a matter of preference.

(Possibly) still to do:

jwallwork23 commented 1 month ago

At the first invocation of cmake build I get a warning that it cannot find FTorch (obviously) for the exercises. Is there any way to suppress this at all?

Please could you share the warning message? I don't think I see such a warning.

During cmake --build . I get a warning about duplicate linking:

ld: warning: ignoring duplicate libraries: '-lemutls_w', '-lgcc'

which would be nice to stop.

Hm, I haven't seen that one. Will look into it.

jwallwork23 commented 1 month ago

It was not immediately clear that I need to go into ./test/examples/examplename/ to invoke ctest. This also feels a bit clunky. I wonder if we could use a shell-script or something to allow users to run the tests from the build or test directory in a one-shot? Feel free to push back if here if you would prefer to get this done and just write clear docs.

Provided a shell script in fe6bbd2, added some text, and hooked it up in the CI test suite. Let me know if it helps.

jwallwork23 commented 1 month ago

Finally, I think there is an element of 'works on my machine' as the regex didn't match on mine causing the tests to fail. Hopefully this can be easily fixed with formatted output...:

@jatkinson1000 please could you rerun these with verbose output and send them over (e.g., in Slack)? So that I can better interpret why the regexes are failing. (For verbose mode, use ctest -V.)

jwallwork23 commented 1 month ago

Okay @jatkinson1000, I added a pages/testing.md page as requested and linked it in elsewhere.

There's a comment above on two new warnings that haven't been addressed. Perhaps we could raise these as issues? Otherwise, ready for re-review.

jwallwork23 commented 1 month ago

Thanks very much for your review @jatkinson1000! Will merge now.