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

Extend FTorch to cover 5-dimensional tensors. #173

Merged
merged 1 commit into from
Oct 28, 2024
Merged

Conversation

jatkinson1000
Copy link
Member

Closes #172

We have had a request for 5D tensor support.
This is from the ConvLSTM: https://paperswithcode.com/method/convlstm which is a type of NN used to analyse gridded data.
Given our initial application (climate sciences) it would make sense to support this as standard.

I have extended the ranks argument in the fypp code to now support 5-D (rank 5) tensors.

One point to discuss is whether we want to support slightly higher in anticipation of other applications (Rank 6 or perhaps 7) bearing in mind that doing so adds quite a lot of code/functions in the f90 file.

We should also perhaps add documentation noting the limit in Rank, and perhaps describing how to extend it for specific cases.

@jatkinson1000
Copy link
Member Author

jatkinson1000 commented Oct 1, 2024

As discussed over in #166 The tests for autograd were producing an error after this change, despite not touching them.

This could be fixed locally on my machine by checking for (.not. associated(out_data)) rather than the shape of unassociated/unallocated out_data.

However, the code is still failing the CI test here.
My guess would be that this is perhaps due to a regex mismatch that is being dealt with over in #166
So question is whether to:

Update: See comment on #166 #166 (comment) and associated PR #174

This seems unrelated to the changes I have made here, so my suggestion is perhaps to turn the autograd test off for now and let this issue be resolved by #166 then turned back on.

@jatkinson1000
Copy link
Member Author

I have just removed the commit that worked around the pointer issues that will be addressed by #175

This should now be merged after:

@jatkinson1000
Copy link
Member Author

Rebased after #175 merged to main.
@TomMelt's changes to regex matching in autograd test as part of #175 mean that #166 is no longer a blocker so this is ready for review.

Copy link
Contributor

@jwallwork23 jwallwork23 left a comment

Choose a reason for hiding this comment

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

Single character change PR, love it!

@jwallwork23 jwallwork23 merged commit 4cf69af into main Oct 28, 2024
6 checks passed
@jwallwork23 jwallwork23 deleted the 5d-tensors branch October 28, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

torch_tensor_from_array cannot convert 5D array into torch tensor
2 participants