Corrected keywords for intrinsic functions #3123

Kishan-Ved · 2024-01-15T14:57:18Z

Fixes: #3076
Credit/Reference: #3105

certik · 2024-01-15T18:08:28Z

Thanks!

Can you please add tests to ensure this change actually works?

Here is the design that we need to move to: #3090

Kishan-Ved · 2024-01-15T18:45:14Z

Sure, I'll start working on it. Thanks!

integration_tests/intrinsics_110.f90

integration_tests/intrinsics_111.f90

integration_tests/intrinsics_112.f90

integration_tests/intrinsics_108.f90

integration_tests/intrinsics_110.f90

Kishan-Ved · 2024-01-17T18:59:40Z

I have corrected the names.

integration_tests/intrinsics_107.f90

certik · 2024-01-17T20:08:11Z

Excellent, I think that starts to look good!

certik · 2024-01-18T15:10:17Z

I think this looks great. Can you please resolve the conflicts? Let's see if tests pass.

Kishan-Ved · 2024-01-18T16:30:26Z

There seems to be some problem with the file: intrinsics_107
It was added by someone else, and hence it appeared as a conflict. I even got an error in it while running integration tests locally, and I fixed it. There are no errors integration test errors on my laptop, but the checks fail due to it.

Kishan-Ved · 2024-01-18T16:34:28Z

For file intrinsics_107:
I got the output on my laptop as:

          32
          32
ERROR STOP

so I changed 32 to 64. But now the Test on GitHub fails, error:

          32
          64
ERROR STOP

certik · 2024-01-18T16:47:42Z

Leave intrinsics_107 as it was (all tests pass at the CI as well as for me locally), just add your new file with a higher number.

HarshitaKalani · 2024-01-18T16:49:11Z

Hey @Kishan-Ved
Please confirm if you followed the same steps
In you branch, after doing

git pull origin main

resolve the conflicts and then do

git clean -dfx
./build0.sh && ./build1.sh

And then update the test references and finally run the integration tests.

I think, you didn't do ./build0.sh && ./build1.sh and that's why: #3123 (comment) issue is coming up.

Kishan-Ved · 2024-01-18T16:51:02Z

Hey @Kishan-Ved Please confirm if you followed the same steps In you branch, after doing
git pull origin main
resolve the conflicts and then do
git clean -dfx
./build0.sh && ./build1.sh
And then update the test references and finally run the integration tests.

I think, you didn't do ./build0.sh && ./build1.sh and that's why: #3123 (comment) issue is coming up.

Will these steps remove my changes?

Kishan-Ved · 2024-01-18T16:54:11Z

Leave intrinsics_107 as it was (all tests pass at the CI as well as for me locally), just add your new file with a higher number.

Done.

Kishan-Ved · 2024-01-18T16:54:43Z

Hey @Kishan-Ved Please confirm if you followed the same steps In you branch, after doing
git pull origin main
resolve the conflicts and then do
git clean -dfx
./build0.sh && ./build1.sh
And then update the test references and finally run the integration tests.
I think, you didn't do ./build0.sh && ./build1.sh and that's why: #3123 (comment) issue is coming up.
Will these steps remove my changes?

But I would like to know this though, how to fix such things without losing my changes?

certik · 2024-01-18T17:34:16Z

integration_tests/intrinsics_107.f90

remove this change

I didn't change this, how do I remove it?

Use

git checkout main -- integration_tests/intrinsics_107.f90 git commit

I did this, but it says there's no such file found

Do git fetch origin first. Ensure origin points to https://github.com/lfortran/lfortran.

Sir I tried that, I still get the error:
(lf) kishan@kishan-IdeaPad-3-15IIL05:~/Desktop/lfortran$ git checkout main -- integration_tests/intrinsics_107.f90 error: pathspec 'integration_tests/intrinsics_107.f90' did not match any file(s) known to git

It's right here: https://github.com/lfortran/lfortran/blob/main/integration_tests/intrinsics_107.f90, so you need to figure out why you don't have it in your local git. You might be in a wrong branch, wrong directory, didn't checkout the latest main, and so on.

Sir I figured this out 👍🏻, I got to learn a new thing today 🙂. Thank you.

integration_tests/CMakeLists.txt

Kishan-Ved · 2024-01-18T17:47:51Z

There is a new file, intrinsics_108, should I change my file to a higher number?

certik · 2024-01-18T17:57:03Z

There is a new file, intrinsics_108, should I change my file to a higher number?

Yes.

certik · 2024-01-18T17:59:02Z

See here https://github.com/lcompilers/lpython/blob/30b5bfc41774833fb94ec0c2e1926ce33be10fdb/doc/src/rebasing.md how to use git for these kinds of rebasing and cleanup tasks.

Kishan-Ved · 2024-01-18T18:16:21Z

I think we should keep x and y in hypot. (According to: https://gcc.gnu.org/onlinedocs/gfortran/HYPOT.html)

HarshitaKalani

This looks good to me. I just left some comments for improvement.

HarshitaKalani · 2024-01-19T06:11:24Z

integration_tests/intrinsics_118.f90

@@ -0,0 +1,3 @@
+program intrinsics_118
+    print *, hypot( x = 1.e0_4, y = 0.5e0_4 )
+end program intrinsics_118


You can add error stop conditions in this file too.

@Kishan-Ved please submit a new PR with this improvement. I already set this one to merge.

Ok Sir. I'll do it.

HarshitaKalani · 2024-01-19T06:14:32Z

integration_tests/intrinsics_120.f90

+    if ( .not. product( array = x, mask = mask ) == 15 ) error stop
+    if ( .not. product( x, mask = mask ) == 15 ) error stop
+    if ( .not. product( x, mask ) == 15 ) error stop
+end


We can follow this for other examples too to ensure that this approach works for all the cases - when we're putting some parameters as arguments directly (line 6) and while some are inserted as keywords(line 4 an line 5).

certik · 2024-01-19T06:16:33Z

Thanks for finishing the PR!

Kishan-Ved added 3 commits January 15, 2024 20:17

corrected keywords for intrinsic functions

d310e42

updated

71e1cfa

identation fixed to 4 spaces

bd6e630

added six tests

948d5d8

HarshitaKalani reviewed Jan 17, 2024

View reviewed changes

updated

48d09ce

certik reviewed Jan 17, 2024

View reviewed changes

integration_tests/intrinsics_107.f90 Outdated Show resolved Hide resolved

all tests added

5da382e

Kishan-Ved requested review from HarshitaKalani and certik January 18, 2024 15:00

identation fixed to 4 spaces

83143e6

Kishan-Ved and others added 2 commits January 18, 2024 21:55

conflicts resolved

6f8d39c

Merge branch 'main' into kv

0cda311

Changed intrinsics_107 back to original state

18d76c0

certik reviewed Jan 18, 2024

View reviewed changes

integration_tests/CMakeLists.txt Outdated Show resolved Hide resolved

updated

af9c457

Kishan-Ved and others added 2 commits January 18, 2024 23:40

resolved conflicts

e5c7d62

Merge branch 'main' into kv

7a2f90d

Kishan-Ved and others added 2 commits January 19, 2024 09:49

Update intrinsics_107.f90

2b4f427

updated

5a7f9fe

HarshitaKalani reviewed Jan 19, 2024

View reviewed changes

certik approved these changes Jan 19, 2024

View reviewed changes

Merge branch 'main' into kv

cf1d769

certik enabled auto-merge (squash) January 19, 2024 06:15

certik merged commit 302c1ab into lfortran:main Jan 19, 2024
21 checks passed

Kishan-Ved mentioned this pull request Jan 19, 2024

Improve intrinsic tests #3145

Merged

This was referenced Jan 29, 2024

Corrected maxloc and minloc min arguments #3106

Closed

Corrected keywords for intrinsic functions #3105

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Corrected keywords for intrinsic functions #3123

Corrected keywords for intrinsic functions #3123

Kishan-Ved commented Jan 15, 2024

certik commented Jan 15, 2024

Kishan-Ved commented Jan 15, 2024

Kishan-Ved commented Jan 17, 2024

certik commented Jan 17, 2024

certik commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

certik commented Jan 18, 2024

HarshitaKalani commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

certik Jan 18, 2024

Kishan-Ved Jan 18, 2024

certik Jan 18, 2024

Kishan-Ved Jan 18, 2024

certik Jan 18, 2024

Kishan-Ved Jan 19, 2024

certik Jan 19, 2024

Kishan-Ved Jan 19, 2024

Kishan-Ved commented Jan 18, 2024

certik commented Jan 18, 2024

certik commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

HarshitaKalani left a comment

HarshitaKalani Jan 19, 2024

certik Jan 19, 2024

Kishan-Ved Jan 19, 2024

HarshitaKalani Jan 19, 2024

certik commented Jan 19, 2024

Corrected keywords for intrinsic functions #3123

Corrected keywords for intrinsic functions #3123

Conversation

Kishan-Ved commented Jan 15, 2024

certik commented Jan 15, 2024

Kishan-Ved commented Jan 15, 2024

Kishan-Ved commented Jan 17, 2024

certik commented Jan 17, 2024

certik commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

certik commented Jan 18, 2024

HarshitaKalani commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kishan-Ved commented Jan 18, 2024

certik commented Jan 18, 2024

certik commented Jan 18, 2024

Kishan-Ved commented Jan 18, 2024

HarshitaKalani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

certik commented Jan 19, 2024