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

Expose APDL Math New Feature - Kronecker product between two matrices/vectors #1950

Merged
merged 9 commits into from
Apr 4, 2023

Conversation

kmkoshy
Copy link
Contributor

@kmkoshy kmkoshy commented Mar 29, 2023

Expose *KRON - Kronecker product (Newly Added feature in V23.2) for APDL Math
Description :
*KRON, M1, M2, M3
Computes the Kronecker product of two matrices/vectors

@kmkoshy kmkoshy self-assigned this Mar 29, 2023
@kmkoshy kmkoshy changed the title Expose APDL Math New Feature - Kronecker product between two matrices… Expose APDL Math New Feature - Kronecker product between two matrices/vectors Mar 29, 2023
@kmkoshy kmkoshy marked this pull request as draft March 29, 2023 09:35
@github-actions github-actions bot added Enhancement Improve any current implemented feature New Feature Request or proposal for a new feature labels Mar 29, 2023
@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Merging #1950 (bc85f99) into main (8e1d1e3) will increase coverage by 85.72%.
The diff coverage is 13.33%.

@@            Coverage Diff            @@
##           main    #1950       +/-   ##
=========================================
+ Coverage      0   85.72%   +85.72%     
=========================================
  Files         0       44       +44     
  Lines         0     7903     +7903     
=========================================
+ Hits          0     6775     +6775     
- Misses        0     1128     +1128     

Copy link
Collaborator

@germa89 germa89 left a comment

Choose a reason for hiding this comment

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

Hi @kmkoshy

Thank you very much for your PR. This is a nice piece of work. Please address whenever you can the comments I left in it. Mainly increase unit testing.

src/ansys/mapdl/core/math.py Outdated Show resolved Hide resolved
src/ansys/mapdl/core/math.py Outdated Show resolved Hide resolved
src/ansys/mapdl/core/math.py Outdated Show resolved Hide resolved
src/ansys/mapdl/core/math.py Outdated Show resolved Hide resolved
src/ansys/mapdl/core/math.py Outdated Show resolved Hide resolved
tests/test_math.py Outdated Show resolved Hide resolved
@kmkoshy
Copy link
Contributor Author

kmkoshy commented Apr 3, 2023

@germa89 Hi German !
I added the suggestions from the review and added more tests to improve coverage. But I see codecov is still failing.
Is this because we can run *KRON only on MAPDL V232 (we skip test after checking MAPDL version ) and we do not have workflows which run on V232 ?

@kmkoshy kmkoshy requested a review from FredAns April 3, 2023 08:49
@FredAns FredAns requested a review from clatapie April 3, 2023 09:51
@germa89
Copy link
Collaborator

germa89 commented Apr 3, 2023

@germa89 Hi German !
I added the suggestions from the review and added more tests to improve coverage. But I see codecov is still failing.
Is this because we can run *KRON only on MAPDL V232 (we skip test after checking MAPDL version ) and we do not have workflows which run on V232 ?

Indeed. It makes me think if we should implement something we cannot check on the CICD. Well, for now, because you run those tests locally, I think it should be fine. Let's approve this and merge.

Can you check if locally the coverage of the new added lines is fine? I presume you need to do that manually.

@kmkoshy kmkoshy marked this pull request as ready for review April 3, 2023 13:41
@kmkoshy kmkoshy merged commit 7a802db into main Apr 4, 2023
1 of 2 checks passed
@kmkoshy kmkoshy deleted the feat/kron-product branch April 4, 2023 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve any current implemented feature New Feature Request or proposal for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants