search

AtChem / AtChem2

Atmospheric chemistry box-model for the MCM
MIT License
58 stars 23 forks source link

Pre-release v1.2 #424

Closed rs028 closed 4 years ago

rs028 commented 4 years ago

This PR includes a few small improvements and bug fixes before the release of 1.2

rs028 commented 4 years ago

Is this all that is needed to solve #407?

codecov[bot] commented 4 years ago

Codecov Report

Merging #424 into master will decrease coverage by 0.17%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #424      +/-   ##
==========================================
- Coverage   88.72%   88.54%   -0.18%     
==========================================
  Files          17       17              
  Lines        2262     2262              
==========================================
- Hits         2007     2003       -4     
- Misses        255      259       +4
Flag Coverage Δ
#build ?
#tests 87.88% <100.00%> (ø)
#unittests 29.26% <100.00%> (-6.51%) :arrow_down:
Impacted Files Coverage Δ
src/argparse.f90 64.74% <ø> (ø)
src/atchem2.f90 90.23% <ø> (ø)
src/atmosphereFunctions.f90 68.96% <ø> (ø)
src/configFunctions.f90 90.19% <ø> (ø)
src/constraintFunctions.f90 81.08% <ø> (ø)
src/dataStructures.f90 95.03% <ø> (ø)
src/inputFunctions.f90 85.73% <ø> (-0.58%) :arrow_down:
src/interpolationFunctions.f90 72.91% <ø> (ø)
src/outputFunctions.f90 89.30% <ø> (ø)
src/parameterModules.f90 100.00% <ø> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 057fdff...9d5ed2f. Read the comment docs.

rs028 commented 4 years ago

Yes I figure alI the fortran files. I started with one to check first that I was not missing something.

rs028 commented 4 years ago

The script mech_converter.py adds implicit none to mechanism.f90 should this be removed as well?

spco commented 4 years ago

We should leave it - it could be moved out of update_p, and into the module mechanism_mod, but up to you whether you move it or leave it where it is.

rs028 commented 4 years ago

Does this solve #407 completely?

spco commented 4 years ago

From what I can grep, that looks right :)

rs028 commented 4 years ago

@spco I would like to solve at least #413, if possible, before merging this. I imagine you are probably swamped with your stuff these days, but would you be able to point me in the right direction?

rs028 commented 4 years ago

It seems I was overly optimistic in thinking I might have more time on my hands during the lockdown :) so I think this is a good point to stop and freeze v1.2.

The build still fails on macOS, but this time with an error message:

/usr/local/Cellar/gcc@4.9/4.9.4_1/bin/gfortran-4.9  selected as fortran compiler, but this is not a valid filename.
Example usage: ./install_cvode.sh /path/to/install/directory /path/to/fortran/compiler
The command "if [[ "$TRAVIS_OS_NAME" == "osx" ]];   then ./tools/install/install_cvode.sh $PWD /usr/local/Cellar/gcc@4.9/4.9.4_1/bin/gfortran-4.9 ; fi" failed and exited with 1 during .

If this is a problem at their end, I will merge it anyways (it is fine on linux).

spco commented 4 years ago

Seems we also have a problem to fix - I've cleared the cache and the install of libffi is still failing. It's probably homebrew changing a setting, or Travis changing something internal with their image. Will need to do some digging as to why that is, but not now - I don't see any reason for it to delay v1.2 either.