Skip to content

Ensure implicit none via compiler option#77

Merged
danielhollas merged 2 commits intomasterfrom
implicit-none
Dec 18, 2021
Merged

Ensure implicit none via compiler option#77
danielhollas merged 2 commits intomasterfrom
implicit-none

Conversation

@danielhollas
Copy link
Copy Markdown
Contributor

@danielhollas danielhollas commented Nov 22, 2021

Adding -fimplicit-none to FFLAGS ensures that we always have implicit none in our modules and functions.
I fixed one missing implicit none (caught an unitialized variable!) and converted one last implicit typing in random.F90.

l also moved mod_const from modules.F90 to a separate file constants.F90 to break circular dependency due to the 'DP' constant. This allowed me reorder compilation of a some files and make 'fatal_error()' accessible to all modules. In general, our goal should be to have one module per file, so while I was in there, I also did the same for mod_files.

Other small changes were done to squash compiler warnings. I also did a bit of cleanup in mass_init and wrote some unit tests to verify this functionality (especially user defined masses code was messy and not fully tested).

Closes #76, Closes #63

@danielhollas danielhollas added the refactor No functional changes, just refactoring or cleaning up the code. label Nov 22, 2021
@danielhollas danielhollas self-assigned this Nov 22, 2021
@danielhollas danielhollas force-pushed the implicit-none branch 2 times, most recently from 989068f to 7c2fc3f Compare November 22, 2021 16:29
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 22, 2021

Codecov Report

Merging #77 (846194e) into master (1618b48) will increase coverage by 0.22%.
The diff coverage is 93.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #77      +/-   ##
==========================================
+ Coverage   69.96%   70.18%   +0.22%     
==========================================
  Files          39       40       +1     
  Lines        5716     5722       +6     
==========================================
+ Hits         3999     4016      +17     
+ Misses       1717     1706      -11     
Impacted Files Coverage Δ
src/error.F90 100.00% <ø> (ø)
src/random.F90 63.67% <ø> (ø)
src/surfacehop.F90 73.87% <ø> (ø)
src/files.F90 90.29% <90.29%> (ø)
src/fftw_interface.F90 97.50% <100.00%> (+0.13%) ⬆️
src/init.F90 66.22% <100.00%> (ø)
src/modules.F90 36.00% <100.00%> (-20.47%) ⬇️
src/tera_mpi_api.F90 98.34% <100.00%> (+0.03%) ⬆️
src/transform.F90 90.60% <100.00%> (+0.14%) ⬆️

@danielhollas danielhollas force-pushed the implicit-none branch 4 times, most recently from 55ec8ff to 54521a7 Compare November 22, 2021 19:15
@danielhollas danielhollas changed the title WIP: ensure implicit none Ensure implicit none via compiler option Nov 22, 2021
@danielhollas danielhollas added the testing Any changes to Github Actions or testing scripts. label Nov 22, 2021
@danielhollas danielhollas force-pushed the implicit-none branch 5 times, most recently from 79a6af8 to de892b1 Compare November 22, 2021 23:17
@danielhollas danielhollas marked this pull request as ready for review November 22, 2021 23:19
@danielhollas danielhollas requested a review from suchanj November 22, 2021 23:19
Comment thread src/files.F90
@@ -0,0 +1,207 @@
! Module for permanent file handling
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No changes here, just moved from module.F90

Comment thread src/random.F90
FUNCTION R1MACH()
IMPLICIT real(DP) (A-H,O-Z)
PARAMETER (ONE=1.D0,TWO=2.D0,HALF=0.5D0)
real(DP), parameter :: ONE=1.D0, TWO=2.D0, HALF=0.5D0
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Last implicit real :-)

Comment thread src/constants.F90
@@ -0,0 +1,38 @@
! Exact values for ANG,AUTOKCAL and AUTODEBYE,me,AUTOM,AUTOFS,AMU:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No changes, just moved from modules.F90

Comment thread src/modules.F90
! https://kiwi.atmos.colostate.edu/fortran/docs/fortran2012.key-8.pdf
! TODO: We should probably move this outside of the modules.f90

! Extra electronic properties (not mandatory)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These were just stubs, not used anywhere, so I removed them for now.

Comment thread src/modules.F90
end do

call init_usermass()
! Check for duplicate user defined atom names
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I inlined init_usermass for simplicity

I think this mode was broken before this commit,
and perhaps it is still broken, but I am not even sure
what it was supposed to do or if it ever worked. Probably not...

I think it was just testing a normal mode transform with PIMD,
using a slightly different version of the hamiltonian.

Closes #63
Also moved mod_const to a separate file
to break circular dependency due to the 'DP' constant.
This let me reorder compilation of some files and make 'fatal_error'
accessible to all modules.

In general, to goal is to have one module per file,
so I also split the mod_files from module.F90 to files.F90

Other small changes were done to squash compiler warnings.

As I was in the modules.F90 anyway,
I also added unit tests for mass initialization
and did a little refactor.
Comment thread configure
"
}

warning_flags="-Wall -Wno-integer-division -Wno-maybe-uninitialized"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To remove some spurious compiler warnings. We should now be warnings-free with this setting I think.

@danielhollas
Copy link
Copy Markdown
Contributor Author

@suchanj thanks for looking at the other PR. 🙇‍♂️ This one is now ready as well, but absolutely no pressure, do on your own time. Thanks! This is a mixture of several small things, see first comment.

Copy link
Copy Markdown
Contributor

@suchanj suchanj left a comment

Choose a reason for hiding this comment

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

Looking good!

@danielhollas danielhollas merged commit 8863f21 into master Dec 18, 2021
@danielhollas danielhollas deleted the implicit-none branch December 18, 2021 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor No functional changes, just refactoring or cleaning up the code. testing Any changes to Github Actions or testing scripts.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use --implicit-none compiler option Compilation with optimalization points to use of uninitialized variables

2 participants