Unit tests (using pFunit) #76

dorchard · 2023-11-28T17:48:45Z

I have started putting in the infrastructure for us to write some unit tests using pFunit.

Create simple unit tests for main functions

Make the cmake portable; I fought a bit, but I don't know enough cmake. We need to be able to link the include directories for ftorch. At the moment the library and include paths need to be specified.
Test torch_tensor_from_blob
Test torch_tensor_from_array
Test torch_tensor_zeros
Test torch_tensor_ones
Test torch_tensor_empty

Other things using torch_module_forward could be better with an integration test since it will need a lot of setup code anyway.

Additionally, we could make use of fixtures and other features of pFUnit in a follow-up.

Closing this would fix #4

dorchard · 2023-11-28T17:55:09Z

tests/test_ftorch.pf gives a start of a unit test for torch_tensor_from_blob

jatkinson1000 · 2024-04-13T21:10:07Z

Has this gone anywhere, or has it stagnated?

As far as I see it tests are the only thing preventing us from making a JOSS submission, and with at least 3 research projects using FTorch that are likely to publish soon this is now a matter of urgency.

For something minimal I was thinking we could have some CMake Test to build and run the SimpleNet example for a few different inputs and check the outputs are correct as an integration test.

dorchard · 2024-04-15T10:28:08Z

There's a start here, but it got a bit stalled as I could use some help with the cmake. Perhaps someone is able to work with me on this? @TomMelt ? Currently has an attempt at using CMake test and does a simple probe of the tensor conversion functions. I agree about running SimpleNet for some integration tests.

jatkinson1000 · 2024-04-15T11:52:54Z

Great, thanks @dorchard
Proposal from the morning is to briefly pause on this in favour of integration tests in #115 so we can make a JOSS submission, then return to this. We can certainly take a look and help with cmake, and the integration tests will use cmake so you might find those useful.

TomMelt · 2024-04-17T06:39:32Z

Hi @dorchard I'm more than happy to help with the CMake side of things. Let me know when is convenient.

jwallwork23 · 2024-11-15T15:57:26Z

.github/workflows/test_suite.yml

+          git clone https://github.com/Goddard-Fortran-Ecosystem/pFUnit.git
+          mkdir pFUnit/build
+          cd pFUnit/build
+          cmake ..


Try -DCMAKE_INSTALL_PREFIX=... to override the build path to avoid version number.

I tried this but it didn't seem to help. Pinning the pFUnit version for now (see fdc4d05).

jwallwork23 · 2024-11-15T17:24:12Z

Perhaps this PR could just set up the infrastructure and add tests for the tensor constructors and we could address writing unit tests for the rest of FTorch in a follow-up PR?

src/test/unit/test_constructors.pf

jwallwork23 · 2024-11-19T10:27:25Z

Interestingly, I needed to set the integer kind to get these tests working on my Ubuntu laptop, too (see c30bc21). I'll update this to follow @TomMelt's 32-bit int approach once #137 is merged.

jwallwork23 · 2024-11-25T10:53:34Z

[Rebased on top of main to bring in linting changes]

jwallwork23 · 2024-12-16T11:34:54Z

Thanks for the tips @TomMelt - with a few tweaks the tests now pass on Ubuntu. There's a build fail in the Windows CI, although I didn't edit the Windows CI workflow or the part of the CMakeLists that it errors at. Do you have any thoughts on what's gone wrong?
https://github.com/Cambridge-ICCS/FTorch/actions/runs/12351579042/job/34466833553?pr=76

jwallwork23 · 2024-12-16T12:02:02Z

Thanks for the tips @TomMelt - with a few tweaks the tests now pass on Ubuntu. There's a build fail in the Windows CI, although I didn't edit the Windows CI workflow or the part of the CMakeLists that it errors at. Do you have any thoughts on what's gone wrong? https://github.com/Cambridge-ICCS/FTorch/actions/runs/12351579042/job/34466833553?pr=76

Huh, I'm getting a similar error on a PR that doesn't change any of the build system or source code (#206): https://github.com/Cambridge-ICCS/FTorch/actions/runs/12352023942/job/34468182561?pr=206

jwallwork23 · 2024-12-18T10:41:26Z

Merged in the Windows fix from #207 in 23ce977. Turned off unit testing for Windows in f1e9add.

jwallwork23 · 2024-12-18T10:48:47Z

@dorchard I can't add you as a reviewer because you authored the initial commits, but any comments you have would be appreciated.

jwallwork23 · 2025-01-06T13:39:12Z

I'm having issues building the unit tests, e.g.:
[ 37%] Building Fortran object test/unit/CMakeFiles/test_constructors.dir/test_constructors.F90.o
/Users/dorchard/Documents/iccs/FTorch/src/build/test/unit/test_constructors.F90:10:7:

   10 |   use pFUnit
      |       1
I am using pfunit-4.10 whose mod files seem to be named funit.mod instead (changing use pFunit to use funit in test_constructors.f90 makes things build...) Has there been a breaking interface change for pfunit? Is it because I built pfUnit skipping the OpenMP and MPI builds I wonder? I can't seem to identify whether this actually changes the mod file though. Maybe I am missing something obvious in the build setup.

Yes, you need to install with MPI to get pFUnit (p for parallel). However, I realised that isn't actually used at present so switched to use FUnit in d52c389 in the current tests, so that should work for you.

jwallwork23 · 2025-01-06T13:55:55Z

Note that I've opened #216 to fix a related issue with torch_tensor_from_array. It will merge into this PR.

dorchard

I think this is a good starting point for our unit tests. I had one suggestion. Having a 3D test would also be good I think, at least on the torch_tensor_from_array side.

src/test/unit/test_constructors.pf

Co-authored-by: Dominic Orchard <[email protected]>

jwallwork23 · 2025-01-06T14:28:31Z

I think this is a good starting point for our unit tests. I had one suggestion. Having a 3D test would also be good I think, at least on the torch_tensor_from_array side.

Good suggestion. I added this in 4dad25e, which required extending the assert_allclose test util to cover the 3D case.

Co-authored-by: Dominic Orchard <[email protected]>

* Call from_blob inside from_array * Accept requires_grad as logical rather than c_bool; test this arg

dorchard · 2025-01-06T15:40:04Z

Looks good to go for me!

TomMelt

Thanks @jwallwork23 and @dorchard , this is great. Really good to get unit tests into Ftorch 🙌

FWIW, I tested locally on my machine and it works great. Only change I had to make was to the CMAKE_PREFIX_PATH as mentioned in my review.

Once that is changed I think its HOTTOGO 🔥 (sorry couldn't resist a Chappell Roan reference)

.github/workflows/test_suite_ubuntu.yml

Co-authored-by: tommelt <[email protected]>

TomMelt

HOTTOGO 🔥

jatkinson1000

Thanks @jwallwork23, this is fantastic.
Since others have run the unit tests and we have been through them together I am going to trust that they are all OK since I can't run locally on my mac right now.

My comments/suggestions focus more on the docs and infrastructure and are minor.

.github/workflows/test_suite_ubuntu.yml

pages/testing.md

.github/workflows/test_suite_ubuntu.yml

src/test/README.md

src/test/unit/CMakeLists.txt

pages/testing.md

src/test/README.md

Co-authored-by: Jack Atkinson <[email protected]>

jwallwork23 · 2025-01-06T17:12:07Z

Thanks again everyone! Merging now 🚀

TomMelt added the enhancement New feature or request label Nov 30, 2023

TomMelt self-requested a review November 30, 2023 11:11

jatkinson1000 mentioned this pull request Apr 15, 2024

Integration Tests #115

Closed

jwallwork23 force-pushed the unit-testing branch from 26219c2 to cf1be0f Compare November 15, 2024 14:15

jwallwork23 reviewed Nov 15, 2024

View reviewed changes

jwallwork23 force-pushed the unit-testing branch 2 times, most recently from c84fee1 to 24ecea6 Compare November 15, 2024 16:41

jwallwork23 assigned dorchard and jwallwork23 Nov 15, 2024

jwallwork23 added the testing Related to FTorch testing label Nov 15, 2024

dorchard commented Nov 18, 2024

View reviewed changes

src/test/unit/test_constructors.pf Outdated Show resolved Hide resolved

jwallwork23 force-pushed the unit-testing branch from 826702a to cf31a70 Compare November 18, 2024 10:29

jwallwork23 force-pushed the unit-testing branch from c30bc21 to d8613fe Compare November 25, 2024 10:53

jwallwork23 force-pushed the unit-testing branch 3 times, most recently from c384f86 to db4de15 Compare December 16, 2024 11:21

jwallwork23 marked this pull request as ready for review December 18, 2024 10:47

jwallwork23 requested a review from jatkinson1000 December 18, 2024 10:47

jwallwork23 added 2 commits January 6, 2025 13:36

Separate 1d and 2d tests for from_array

ad67a0e

FUnit is sufficient for now

d52c389

switch rest of the imports to FUnit

b96fece

jwallwork23 mentioned this pull request Jan 6, 2025

Call from_blob inside from_array #216

Merged

Drop unused variable

a413d7b

dorchard commented Jan 6, 2025

View reviewed changes

src/test/unit/test_constructors.pf Show resolved Hide resolved

src/test/unit/test_constructors.pf Outdated Show resolved Hide resolved

src/test/unit/test_constructors.pf Outdated Show resolved Hide resolved

jwallwork23 added the autograd Tasks towards the online training / automatic differentiation feature label Jan 6, 2025

jwallwork23 and others added 3 commits January 6, 2025 14:19

Apply suggestion from @dorchard code review

42f3451

Co-authored-by: Dominic Orchard <[email protected]>

Check array shapes

07ab337

Add 3D unit test for torch_tensor_from_array

4dad25e

jwallwork23 and others added 2 commits January 6, 2025 14:58

Apply suggestions from @dorchard code review

217f31a

Co-authored-by: Dominic Orchard <[email protected]>

Call from_blob inside from_array (#216)

f24db4d

* Call from_blob inside from_array * Accept requires_grad as logical rather than c_bool; test this arg

jwallwork23 mentioned this pull request Jan 6, 2025

Reorganise main mod #217

Merged

TomMelt requested changes Jan 6, 2025

View reviewed changes

.github/workflows/test_suite_ubuntu.yml Outdated Show resolved Hide resolved

.github/workflows/test_suite_ubuntu.yml Show resolved Hide resolved

.github/workflows/test_suite_ubuntu.yml Outdated Show resolved Hide resolved

Apply suggestions from @TomMelt code review

cbdefb3

Co-authored-by: tommelt <[email protected]>

jwallwork23 requested a review from TomMelt January 6, 2025 15:56

TomMelt approved these changes Jan 6, 2025

View reviewed changes

jatkinson1000 approved these changes Jan 6, 2025

View reviewed changes

jwallwork23 mentioned this pull request Jan 6, 2025

DO NOT MERGE: Test pFUnit install without MPI=YES #218

Closed

jatkinson1000 reviewed Jan 6, 2025

View reviewed changes

pages/testing.md Outdated Show resolved Hide resolved

jatkinson1000 reviewed Jan 6, 2025

View reviewed changes

src/test/README.md Show resolved Hide resolved

jwallwork23 and others added 2 commits January 6, 2025 17:03

Apply suggestions from @jatkinson1000 code review

80c1d18

Co-authored-by: Jack Atkinson <[email protected]>

Apply other suggestions from @jatkinson1000 code review

ef4615e

Co-authored-by: Jack Atkinson <[email protected]>

jwallwork23 merged commit 6055a7d into main Jan 6, 2025
6 checks passed

jwallwork23 deleted the unit-testing branch January 6, 2025 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unit tests (using pFunit) #76

Unit tests (using pFunit) #76

dorchard commented Nov 28, 2023 •

edited by jwallwork23

Loading

dorchard commented Nov 28, 2023

jatkinson1000 commented Apr 13, 2024

dorchard commented Apr 15, 2024 •

edited

Loading

jatkinson1000 commented Apr 15, 2024 •

edited

Loading

TomMelt commented Apr 17, 2024

jwallwork23 Nov 15, 2024 •

edited

Loading

jwallwork23 Nov 15, 2024

jwallwork23 commented Nov 15, 2024

jwallwork23 commented Nov 19, 2024

jwallwork23 commented Nov 25, 2024

jwallwork23 commented Dec 16, 2024

jwallwork23 commented Dec 16, 2024

jwallwork23 commented Dec 18, 2024

jwallwork23 commented Dec 18, 2024

jwallwork23 commented Jan 6, 2025 •

edited

Loading

jwallwork23 commented Jan 6, 2025

dorchard left a comment

jwallwork23 commented Jan 6, 2025

dorchard commented Jan 6, 2025

TomMelt left a comment

TomMelt left a comment

jatkinson1000 left a comment

jwallwork23 commented Jan 6, 2025

Unit tests (using pFunit) #76

Unit tests (using pFunit) #76

Conversation

dorchard commented Nov 28, 2023 • edited by jwallwork23 Loading

dorchard commented Nov 28, 2023

jatkinson1000 commented Apr 13, 2024

dorchard commented Apr 15, 2024 • edited Loading

jatkinson1000 commented Apr 15, 2024 • edited Loading

TomMelt commented Apr 17, 2024

jwallwork23 Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

jwallwork23 Nov 15, 2024

Choose a reason for hiding this comment

jwallwork23 commented Nov 15, 2024

jwallwork23 commented Nov 19, 2024

jwallwork23 commented Nov 25, 2024

jwallwork23 commented Dec 16, 2024

jwallwork23 commented Dec 16, 2024

jwallwork23 commented Dec 18, 2024

jwallwork23 commented Dec 18, 2024

jwallwork23 commented Jan 6, 2025 • edited Loading

jwallwork23 commented Jan 6, 2025

dorchard left a comment

Choose a reason for hiding this comment

jwallwork23 commented Jan 6, 2025

dorchard commented Jan 6, 2025

TomMelt left a comment

Choose a reason for hiding this comment

TomMelt left a comment

Choose a reason for hiding this comment

jatkinson1000 left a comment

Choose a reason for hiding this comment

jwallwork23 commented Jan 6, 2025

dorchard commented Nov 28, 2023 •

edited by jwallwork23

Loading

dorchard commented Apr 15, 2024 •

edited

Loading

jatkinson1000 commented Apr 15, 2024 •

edited

Loading

jwallwork23 Nov 15, 2024 •

edited

Loading

jwallwork23 commented Jan 6, 2025 •

edited

Loading