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

feat: add C and Fortran implementation for blas/base/zaxpy #4215

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

gururaj1512
Copy link
Member

@gururaj1512 gururaj1512 commented Dec 25, 2024

Resolves a part of #2039.

Description

What is the purpose of this pull request?

This pull request:

  • Adds C and fortran implementation for blas/base/zaxpy

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
@stdlib-bot stdlib-bot added BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). Needs Review A pull request which needs code review. labels Dec 25, 2024
@gururaj1512
Copy link
Member Author

@kgryte, @aman-095, how can i resove below error
65 | intrinsic dcabs1 | 1 Error: ‘dcabs1’ declared INTRINSIC at (1) does not exist

@kgryte
Copy link
Member

kgryte commented Dec 25, 2024

@gururaj1512 Yes, this is a known issue. Namely, our tooling needs to be updated to handle Fortran dependencies. Until then, adding Fortran implementations for packages, such as zaxpy, is blocked.

@kgryte kgryte added the status: Blocked Issue or pull request which is current blocked. label Dec 25, 2024
@gururaj1512
Copy link
Member Author

@kgryte, Can't we solve it by

function dcabs1(z) result(res)
    implicit none
    complex(kind=kind(0.0d0)), intent(in) :: z
    real(kind=kind(0.0d0)) :: res
    
    res = abs(real(z)) + abs(aimag(z))
end function dcabs1

@kgryte
Copy link
Member

kgryte commented Dec 25, 2024

@gururaj1512 That could be a quick fix, but it would not solve the larger issue for other packages which also rely on Fortran functions defined in other packages. In those instances, we need to the resolution of package Fortran files and compiling together. We can already do this for C, but we've yet to update to tooling to appropriately handle Fortran source files. In this case, we've added a dcabs package, but are currently unable to use it within other BLAS routines.

@kgryte
Copy link
Member

kgryte commented Dec 25, 2024

I'm fine with doing something similar to your suggestion as a quick fix for this package with a FIXME indicating that this needs to be patched once we've fixed project tooling.

@gururaj1512
Copy link
Member Author

@kgryte, Should I proceed with quick fix with a FIXME tag for now to keep this package functional or we can proceed once the Fortran tooling integration is improved. Let me know if there's anything specific you'd like me to include in the comment or implementation.

@kgryte
Copy link
Member

kgryte commented Dec 25, 2024

@gururaj1512 Sure, feel free to proceed. As for comment, "FIXME: replace with blas/base/dcabs1 once library manifest tooling is fixed".

@gururaj1512
Copy link
Member Author

Thank you for clarifying!

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: failed
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: failed
---
@gururaj1512
Copy link
Member Author

@kgryte, Are there any commands to test fortran implementation before pushing the code.

@kgryte
Copy link
Member

kgryte commented Dec 26, 2024

The way we test all native implementations is via JavaScript. Namely, you need to compile the native add-on against the desired set of source files. Locally, you can tweak the manifest.json file such that, based on your OS, project tooling resolves the correct set of source files. Note, however, that if you are on Windows, you likely won't have a Fortran compiler installed. And I am not sure how well, say, GFortran works under WSL (Linux subsystem for Windows).

@kgryte
Copy link
Member

kgryte commented Dec 26, 2024

If you want to know which source files are being resolved during compilation of the native add-on, you can set the DEBUG environment variable to *. E.g.,

DEBUG=* make install-node-addons NODE_ADDONS_PATTERN="blas/base/zaxpy"

@gururaj1512
Copy link
Member Author

The way we test all native implementations is via JavaScript. Namely, you need to compile the native add-on against the desired set of source files. Locally, you can tweak the manifest.json file such that, based on your OS, project tooling resolves the correct set of source files. Note, however, that if you are on Windows, you likely won't have a Fortran compiler installed. And I am not sure how well, say, GFortran works under WSL (Linux subsystem for Windows).

GFortran is not working well, at least on my machine. I am trying to set up Linux on my machine to move forward.

---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
  - task: lint_filenames
    status: passed
  - task: lint_editorconfig
    status: passed
  - task: lint_markdown
    status: na
  - task: lint_package_json
    status: na
  - task: lint_repl_help
    status: na
  - task: lint_javascript_src
    status: na
  - task: lint_javascript_cli
    status: na
  - task: lint_javascript_examples
    status: na
  - task: lint_javascript_tests
    status: na
  - task: lint_javascript_benchmarks
    status: na
  - task: lint_python
    status: na
  - task: lint_r
    status: na
  - task: lint_c_src
    status: na
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: na
  - task: lint_c_tests_fixtures
    status: na
  - task: lint_shell
    status: na
  - task: lint_typescript_declarations
    status: na
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
  - task: lint_filenames
    status: passed
  - task: lint_editorconfig
    status: passed
  - task: lint_markdown
    status: na
  - task: lint_package_json
    status: na
  - task: lint_repl_help
    status: na
  - task: lint_javascript_src
    status: passed
  - task: lint_javascript_cli
    status: na
  - task: lint_javascript_examples
    status: na
  - task: lint_javascript_tests
    status: passed
  - task: lint_javascript_benchmarks
    status: na
  - task: lint_python
    status: na
  - task: lint_r
    status: na
  - task: lint_c_src
    status: na
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: na
  - task: lint_c_tests_fixtures
    status: na
  - task: lint_shell
    status: na
  - task: lint_typescript_declarations
    status: na
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: passed
---
@stdlib-bot
Copy link
Contributor

stdlib-bot commented Dec 29, 2024

Coverage Report

Package Statements Branches Functions Lines
blas/base/zaxpy 471 / 471
+ 100.00 %
19 / 19
+ 100.00 %
4 / 4
+ 100.00 %
471 / 471
+ 100.00 %

The above coverage report was generated for the changes in this PR.

---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
  - task: lint_filenames
    status: passed
  - task: lint_editorconfig
    status: passed
  - task: lint_markdown
    status: na
  - task: lint_package_json
    status: na
  - task: lint_repl_help
    status: na
  - task: lint_javascript_src
    status: passed
  - task: lint_javascript_cli
    status: na
  - task: lint_javascript_examples
    status: na
  - task: lint_javascript_tests
    status: na
  - task: lint_javascript_benchmarks
    status: na
  - task: lint_python
    status: na
  - task: lint_r
    status: na
  - task: lint_c_src
    status: missing_dependencies
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: na
  - task: lint_c_tests_fixtures
    status: na
  - task: lint_shell
    status: na
  - task: lint_typescript_declarations
    status: na
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
  - task: lint_filenames
    status: passed
  - task: lint_editorconfig
    status: passed
  - task: lint_markdown
    status: na
  - task: lint_package_json
    status: na
  - task: lint_repl_help
    status: na
  - task: lint_javascript_src
    status: na
  - task: lint_javascript_cli
    status: na
  - task: lint_javascript_examples
    status: na
  - task: lint_javascript_tests
    status: na
  - task: lint_javascript_benchmarks
    status: na
  - task: lint_python
    status: na
  - task: lint_r
    status: na
  - task: lint_c_src
    status: na
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: missing_dependencies
  - task: lint_c_tests_fixtures
    status: na
  - task: lint_shell
    status: na
  - task: lint_typescript_declarations
    status: na
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: passed
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
@gururaj1512
Copy link
Member Author

@kgryte, Instead of

function dcabs1(z) result(res)
    implicit none
    complex(kind=kind(0.0d0)), intent(in) :: z
    real(kind=kind(0.0d0)) :: res
    
    res = abs(real(z)) + abs(aimag(z))
end function dcabs1

I have used

! Intrinsic functions:
intrinsic abs
if ( ( abs(real(za)) + abs(aimag(za)) ) <= 0.0D0 ) then
    return
end if

Which is working fine. PR ready for review

---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
  - task: lint_filenames
    status: passed
  - task: lint_editorconfig
    status: passed
  - task: lint_markdown
    status: na
  - task: lint_package_json
    status: na
  - task: lint_repl_help
    status: na
  - task: lint_javascript_src
    status: na
  - task: lint_javascript_cli
    status: na
  - task: lint_javascript_examples
    status: na
  - task: lint_javascript_tests
    status: passed
  - task: lint_javascript_benchmarks
    status: na
  - task: lint_python
    status: na
  - task: lint_r
    status: na
  - task: lint_c_src
    status: na
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: na
  - task: lint_c_tests_fixtures
    status: na
  - task: lint_shell
    status: na
  - task: lint_typescript_declarations
    status: na
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: passed
---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). Needs Review A pull request which needs code review. status: Blocked Issue or pull request which is current blocked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants