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

Updates in Lapack Utility #311

Merged
merged 2 commits into from
May 18, 2024
Merged

Updates in Lapack Utility #311

merged 2 commits into from
May 18, 2024

Conversation

shishiousan
Copy link
Member

  • adding GE_EigenValueMethods

- adding GE_EigenValueMethods
@shishiousan shishiousan added enhancement New feature or request linalg linear algebra labels May 17, 2024
@shishiousan shishiousan self-assigned this May 17, 2024
Copy link
Member

@vickysharma0812 vickysharma0812 left a comment

Choose a reason for hiding this comment

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

@shishiousan instead of COMPLEX(DFP) please use COMPLEX(DFPC) (this is a minor correction)

Copy link
Member

@vickysharma0812 vickysharma0812 left a comment

Choose a reason for hiding this comment

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

@shishiousan
instead of using array with intent(out), use intent(inout). In this case, you can get the size of output array (which is allocated by the user before calling the routine). If you use intent(out) on an array, you can not get correct information of array size (even if it is allocated by user before calling the routine)

Copy link
Member

@vickysharma0812 vickysharma0812 left a comment

Choose a reason for hiding this comment

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

@shishiousan Please change display to Display (functions and subroutine should start from upper case) (This is a minor issue)

- minor changes
@shishiousan
Copy link
Member Author

@vickysharma0812
Thank you for your review. I corrected. Please check.

src/submodules/Lapack/src/GE_EigenValueMethods@Methods.F90 Outdated Show resolved Hide resolved
src/submodules/Lapack/src/GE_EigenValueMethods@Methods.F90 Outdated Show resolved Hide resolved
src/submodules/Lapack/src/GE_EigenValueMethods@Methods.F90 Outdated Show resolved Hide resolved
lrwork = 2 * n
ALLOCATE (At(lda, n), vl(ldvl, n), vr(ldvr, n), &
work(lwork), rwork(lrwork))
At = A
Copy link
Member

Choose a reason for hiding this comment

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

Why do you make a copy

Copy link
Member Author

@shishiousan shishiousan May 17, 2024

Choose a reason for hiding this comment

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

@vickysharma0812
This is because GEEV destroy the A
I can add optional boolean argument like destroy to switch making copy.

For example

INTERFACE GetEigVals
  MODULE SUBROUTINE deigvals(A, lam, destroy)
    REAL(DFP), INTENT(IN) :: A(:, :)
    COMPLEX(DFPC), INTENT(INOUT) :: lam(:)
    LOGICAL(LGT), OPTIONAL, INTENT(IN) :: destroy
    ! default true
  END SUBROUTINE deigvals
END INTERFACE GetEigVals

How do you think ?

Copy link
Member

@vickysharma0812 vickysharma0812 left a comment

Choose a reason for hiding this comment

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

@shishiousan Awesome 👍

@vickysharma0812 vickysharma0812 merged commit d4d84a6 into dev May 18, 2024
@vickysharma0812 vickysharma0812 deleted the eigen branch May 18, 2024 09:08
@shishiousan shishiousan restored the eigen branch May 18, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request linalg linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants