Skip to content

Add tests of the Fortran module#874

Merged
speth merged 6 commits intoCantera:masterfrom
speth:fortran-tests
Jun 24, 2020
Merged

Add tests of the Fortran module#874
speth merged 6 commits intoCantera:masterfrom
speth:fortran-tests

Conversation

@speth
Copy link
Copy Markdown
Member

@speth speth commented Jun 20, 2020 •
edited
Loading

Changes proposed in this pull request

  • Add tests of the Fortran module, using the existing F77 and F90 "samples"
  • Fix some issues to enable compilation using Flang on Windows (installed from conda-forge)

Checklist

  • There is a clear use-case for this code change
  • The commit message has a short title & references relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • The pull request is ready for review

@bryanwweber
Copy link
Copy Markdown
Member

bryanwweber commented Jun 20, 2020

Woo! Fortran! A small comment before my brain turns off completely - the change in 9e4ba1c about path separators appears as though it will be backwards-incompatible with cantera.conf files that people might already have. Is there any way to issue warnings/errors, particularly on Windows?

@speth
Copy link
Copy Markdown
Member Author

speth commented Jun 20, 2020

I don't think it should be backwards-incompatible where it is currently works (Linux / macOS), and on Windows the old behavior was broken, so we don't need to worry about the change in behavior there, right?

@bryanwweber
Copy link
Copy Markdown
Member

bryanwweber commented Jun 20, 2020

Yeah, I agree about macOS and Linux. Was it broken on Windows such that the user would get an error message and the build would stop? In that case, we don't need to worry I guess. But if it was able to work with colons instead of semicolons at all (in other words, no error message even if it ended up doing something incorrect), then I think it would be breaking in the sense that there is now a behavior change for the user (to the correct behavior, but still...).

@speth
Copy link
Copy Markdown
Member Author

speth commented Jun 20, 2020

I added an error message to what I think is the only case where using a colon as a path separator in extra_lib_dirs would have previously worked. The behavior I was seeing is that if you have given a path like c:\some\path, SCons was adding <current directory>\c and <current drive>\some\path as library directories. Which "works" as long as everything is on the same drive.

speth added 3 commits June 20, 2020 12:21
@speth
[Fortran/Samples] Use MSVC to link Fortran samples
08a6a68 
When using the MSVC toolchain together with Fortran compilers like Flang,
MSVC should be used as the linker.
@speth
[Fortran] Make output from Fortran samples more consistent
33f8210 
Some compilers (including Flang) output the message:

    "Warning: ieee_inexact is signaling"

when running the Fortran sample programs. This output is a consequence of
F2003 standard compliance by some compilers, and can be avoided by letting
the program exit normally rather than using the 'stop' statement.
@speth
[SCons] Add general variable for C++ standard library
5f2a439
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 20, 2020 •
edited
Loading

Codecov Report

Merging #874 into master will decrease coverage by 1.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #874      +/-   ##
==========================================
- Coverage   71.56%   70.54%   -1.02%     
==========================================
  Files         372      375       +3     
  Lines       44510    45349     +839     
==========================================
+ Hits        31853    31993     +140     
- Misses      12657    13356     +699
Impacted Files Coverage Δ
src/fortran/fct.cpp 16.81% <0.00%> (ø)
samples/f77/demo_ftnlib.cpp 31.85% <0.00%> (ø)
src/fortran/fctxml.cpp 0.00% <0.00%> (ø)
src/thermo/ThermoPhase.cpp 69.84% <0.00%> (+0.46%) ⬆️

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 4bf3baa...b0eb45c. Read the comment docs.

speth added 2 commits June 24, 2020 14:47
@speth
[Fortran/Test] Add regression tests using the Fortran samples
29dcf3a 
The changes in the ctlib blessed output are mainly due to the updates of
physical constants and atomic weights (see 3e4842b and dc96fb5).
@speth
[SCons] Use OS-specific path separator in extra_lib_dirs
b0eb45c
bryanwweber
bryanwweber approved these changes Jun 24, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

Thanks for adding the warning

@speth speth merged commit d321abe into Cantera:master Jun 24, 2020
@speth speth mentioned this pull request Jun 27, 2020
Merged
4 tasks
@speth speth deleted the fortran-tests branch June 29, 2020 02:28
@bryanwweber bryanwweber mentioned this pull request Jul 3, 2020
Closed
4 tasks
@ischoegl ischoegl mentioned this pull request Jul 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bryanwweber bryanwweber bryanwweber approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@speth @bryanwweber