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

[WIP] broke getGlobalRowView #216

Closed

Conversation

tjfulle
Copy link
Collaborator

@tjfulle tjfulle commented Feb 12, 2018

No description provided.

@codecov-io
Copy link

codecov-io commented Feb 19, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@71600fd). Click here to learn what that means.
The diff coverage is 82.03%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #216   +/-   ##
=========================================
  Coverage          ?   34.38%           
=========================================
  Files             ?       33           
  Lines             ?    13444           
  Branches          ?        0           
=========================================
  Hits              ?     4623           
  Misses            ?     8821           
  Partials          ?        0
Impacted Files Coverage Δ
src/tpetra/test/test_tpetra_crsmatrix.f90 100% <100%> (ø)
src/tpetra/src/fortpetraFORTRAN_wrap.cxx 21.21% <61.22%> (ø)
src/tpetra/src/fortpetra.f90 31.94% <71.42%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71600fd...b130163. Read the comment docs.


! check for throws and no-throws/values
TEST_THROW(cgview => A%getGlobalRowIndicesView(gblrow))
TEST_THROW(csview => A%getGlobalRowValuesView(gblrow))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@aprokop, to be consistent with Tpetra, we could make a non-member getGlobalRowView procedure that mimics the C++ member version. Something like:

subroutine getGlobalRowView(A, gblrow, cgview, csview)
   type(TpetraCrsMatrix), intent(in) :: A
   integer(global_ordinal_type), intent(in) :: gblrow
   real(scalar_type), dimension(:), pointer :: csview
   integer(global_ordinal_type), dimension(:), pointer :: cgview
  cgview => A%getGlobalRowIndicesView(gblrow)
  csview => A%getGlobalRowValuesView(gblrow)
end subroutine

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, ok. @sethrj Is it possible to inject code fragment into the Fortran wrapper for a specific class?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, there are several ways it could be done. If the function signature can be automatically converted correctly from C++ then it's best to use %extend on the class, then you can add code to the proxy function with %fortranappend or %fortranprepend. Or, you can replace the function call in the proxy code entirely using %feature("shadow").

The most manual solution is to extend the fortran class by manually extending the proxy code. I think it would look like this: one fragment adds to the module code, the second injects code into the class type definition.

%extend TpetraCrsMatrix {
%proxycode {
subroutine tpetracrsmat_getGlobalRowView(A, gblrow, cgview, csview)
   type(TpetraCrsMatrix), intent(in) :: A
   integer(global_ordinal_type), intent(in) :: gblrow
   real(scalar_type), dimension(:), pointer :: csview
   integer(global_ordinal_type), dimension(:), pointer :: cgview
  cgview => A%getGlobalRowIndicesView(gblrow)
  csview => A%getGlobalRowValuesView(gblrow)
end subroutine
}
%insert("ftypes") {
getGlobalRowView => tpetracrsmat_getGlobalRowView
}
}

@aprokop aprokop self-assigned this Apr 11, 2018
@aprokop aprokop changed the base branch from develop to master June 7, 2018 15:03
@sethrj
Copy link
Collaborator

sethrj commented Feb 5, 2019

The workaround I posted is not valid with the latest SWIG versions.

I've updated the contents of @tjfulle's test and pushed to a new branch at https://github.com/sethrj/ForTrilinos/tree/fix-getGlobalRowView . It looks like the test now fails to compile, so that's a plus. I think it'll just be a simple matter of creating a new SWIGTYPE for "arrayview by reference".

@sethrj
Copy link
Collaborator

sethrj commented Aug 17, 2022

Closing due to inactivity

@sethrj sethrj closed this Aug 17, 2022
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.

4 participants