Reference-LAPACK
Standardized documentation
Description
This PR updates only the documentation, more specifically the PARAMS section. In all cases, I tried to standardize the comment style, i.e. follow the majority. Changes include
- dimensions (X)
+ dimension (X)
- dimension X
+ dimension (X)
the aim of these changes is to simplify the creation of Fortran interfaces for type checking during compilation. With the current changes, it is possible to create interfaces based on the documentation for approx 1700/2000 routines.
The script and the interfaces are attached below (with obfuscated file extension) create_interface_py.txt
Checklist
- [x] The documentation has been updated.
More standardized documentation is something I can always get behind. Some comments:
- Are the interfaces generated by ifort not a more sustainable solution? They are also more safe in case of a mismatch between the declaration and documentation.
- If we want to offer an interface module to users of LAPACK, I suggest we include it in this repository. It shouldn't be too hard to generate this automatically using the CI. This is better for our users because they can just download a single interface file and also better for the maintainers because the generation can easily be changed and errors will be noticed quickly.
Correction: gfortran allows passing different shape arrays even when declared explicitly, it simply checks the total size.
Are the interfaces generated by ifort not a more sustainable solution? They are also more safe in case of a mismatch between the declaration and documentation.
Technically yes. However, as far as I see, in the actual code the dimension is always given as dimension(*) which is not sufficient to avoid out of bounds access. The long-term solution would be to specify the dimensions in the actual function signature and put the whole code into a module. That would of course break backward compatibility.
If we want to offer an interface module to users of LAPACK, I suggest we include it in this repository. It shouldn't be too hard to generate this automatically using the CI. This is better for our users because they can just download a single interface file and also better for the maintainers because the generation can easily be changed and errors will be noticed quickly.
Agreed. If there is interest, I will share the script and contribute to the integration. However, I don't know if the CI works if the interfaces should be part of the repository.
Correction: gfortran allows passing different shape arrays even when declared explicitly, it simply checks the total size.
yes, it only refuses to compile if the total number of elements is smaller than required.
Agreed. If there is interest, I will share the script and contribute to the integration. However, I don't know if the CI works if the interfaces should be part of the repository.
The interfaces shouldn't change that much, so we can commit them and have the CI compare the generated and committed versions. Similar to a snapshot test.
Technically yes. However, as far as I see, in the actual code the dimension is always given as dimension(*) which is not sufficient to avoid out of bounds access. The long-term solution would be to specify the dimensions in the actual function signature and put the whole code into a module. That would of course break backward compatibility.
Maybe I've just been indoctrinated into the old ways, but I don't think specifying the dimensions in the actual function signature would be a good idea for LAPACK. Explicit shape arrays are to my knowledge always contiguous so that might lead to some extra copies. It also makes linking with C more difficult. I can't say whether this interface will also lead to copy in/copy out as it would when defining the shape in the source, but it wouldn't surprise me. This will require more investigation. EDIT: If my investigation is correct, non-contiguous arrays will lead to copy-in/copy-out with assumed size syntax anyway, so this is not necessarily a disadvantage of explicit shape.
Agreed. If there is interest, I will share the script and contribute to the integration. However, I don't know if the CI works if the interfaces should be part of the repository.
The interfaces shouldn't change that much, so we can commit them and have the CI compare the generated and committed versions. Similar to a snapshot test.
Sounds good, I'll see what I can do. Certainly a lot of testing is still needed from my side.
Technically yes. However, as far as I see, in the actual code the dimension is always given as dimension(*) which is not sufficient to avoid out of bounds access. The long-term solution would be to specify the dimensions in the actual function signature and put the whole code into a module. That would of course break backward compatibility.
Maybe I've just been indoctrinated into the old ways, but I don't think specifying the dimensions in the actual function signature would be a good idea for LAPACK. Explicit shape arrays are to my knowledge always contiguous so that might lead to some extra copies. It also makes linking with C more difficult. I can't say whether this interface will also lead to copy in/copy out as it would when defining the shape in the source, but it wouldn't surprise me. This will require more investigation. EDIT: If my investigation is correct, non-contiguous arrays will lead to copy-in/copy-out with assumed size syntax anyway, so this is not necessarily a disadvantage of explicit shape. Good to hear. I have not looked into interfacing with C in detail, but I believe with modern Fortran it is possible.
2 more problems I see with the interfaces:
- Some declarations of the workspace array in the interface are DIMENSION(LWORK), this can error during the workspace query
- The interface causes problems for optional arguments. For example, in DGEEV, if no eigenvectors are required, you can supply any array to VL and it doesn't have to be the correct size because it is not referenced. If the interface makes the dimensions explicit, this is no longer possible.
2 more problems I see with the interfaces:
* Some declarations of the workspace array in the interface are DIMENSION(LWORK), this can error during the workspace query
in this case, LWORK can't be intent(in) and it is not able to use it to the specify the dimension of other aguments.
* The interface causes problems for optional arguments. For example, in DGEEV, if no eigenvectors are required, you can supply any array to VL and it doesn't have to be the correct size because it is not referenced. If the interface makes the dimensions explicit, this is no longer possible.
yes, the better approach would be to mark it as optional.