implemented clip function

[aman-godara]

Status: Code written is open for review
Regarding: #319
Provided Specifications: checkout #319 (comment)

Status:

• Implemented clip function in stdlib_math.fypp file
• added both invalid & valid tests for clip function in test_stdlib_math.f90 file (this .f90 file was created using fypp preprocessor from test_math.fypp file)
• Make sure the new module builds and tests out using CMakefile builds.
• Documented clip function in newly created stdlib_math.md file.
• Make sure the new module builds and tests out using Makefile builds.

[milancurcic]

Thanks @aman-godara. Please write your questions and comments in the issue thread (here) instead of the source code.

[aman-godara]

I have created the 7 functions as requested in #319 (comment) i.e. there is a function for each of the int8, int16, .... , real128 kinds.

Now for my query consider a case where xmin is (let's say) int8 and xmax is (let's say) int16. Now xmin and xmax both belong to the same type (integer) but different kind (int8 and int16 respectively). Will this case be automatically handled by fortran somehow (may be by converting xmin to int16) or should I write a function (similar to the 7 functions that I have already created) to handle a case like this?

[milancurcic]

Will this case be automatically handled by fortran somehow (may be by converting xmin to int16) or should I write a function (similar to the 7 functions that I have already created) to handle a case like this?

It won't be handled automatically. You'd need to write specific functions for each combination of types and kinds. For integer variants that would be 4^3 = 64 specific functions and for real variants that would be 3^3 = 27 specific functions. And it gets worse if you want to do something like clip(real, int, int).

I don't think it's worth it. Let's simply require all three arguments to be of the same type and kind.

[awvwgk]

Note that you haven't included the new module with the CMake build system yet:

stdlib/src/CMakeLists.txt

Lines 3 to 5 in 8a5f3d4

	# Create a list of the files to be preprocessed
	set(fppFiles
	stdlib_bitsets.fypp

As well as the manual Makefiles here:

stdlib/src/Makefile.manual

Lines 1 to 2 in 8a5f3d4

	SRCFYPP =\
	stdlib_bitsets_64.fypp \

The module will not be preprocessed and compiled with the current setup.

[aman-godara]

Let's simply require all three arguments to be of the same type and kind.

Should I write some validation check in the function to ensure that user gets a warning whenever same type and same kind condition is violated OR will fortran throw an error automatically?

[milancurcic]

Should I write some validation check in the function to ensure that user gets a warning whenever same type and same kind condition is violated OR will fortran throw an error automatically?

There will be a compile-time error.

[aman-godara]

Should I continue to submit test_math.fypp file directly (currently PR has test_math.fypp file)
OR
Should I submit the .f90 version of test_math.fypp file (which can be created by fypp test_math.fypp test_math.f90 command) since ADDTEST doesn't support .fypp files?

Let me make some updates to demonstrate what I am proposing.

[ivan-pi]

Just a few minor suggestions, but otherwise looks good to me.

[milancurcic]

Looking at the test_math.f90 in more detail now, I'm confused why you chose to wrap calls to check() in subroutines that don't seem to do anything else. For example, you have:

 subroutine test_clip_int8(x, xmin, xmax, compare) integer(int8), intent(in) :: x, xmin, xmax, compare call check(clip(x, xmin, xmax) == compare) end subroutine test_clip_int8 

and then the same exact subroutine for other type kinds. Then you do

call test_clip_int8(2_int8, -2_int8, 5_int8, 2_int8) 

But why not just do

call check(clip(2_int8, -2_int8, 5_int8) == 2_int8) 

and get rid of the unnecessary boilerplate? Further, you can now do:

call check(clip(2_int8, -2_int8, 5_int8) == 5_int8, 'Test clip_int8 failed.', warn=.true.) 

which will print a helpful message (while not stopping the program) if some of the tests fail.

[milancurcic]

I think this is good to go, thank you Aman and reviewers!

[milancurcic]

We still need one more approval to merge this.

[awvwgk]

This looks good to get merged.

Requests were addressed

[milancurcic]

Thank you @aman-godara and reviewers!