Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add in a C++ linter #555

Merged
merged 26 commits into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
00cfdc5
Move soca utils to subdirectory
CoryMartin-NOAA Aug 7, 2023
072b63d
print out nlocs
CoryMartin-NOAA Aug 7, 2023
0741a92
Working example; need test now
CoryMartin-NOAA Aug 7, 2023
ca3d01f
Some placeholders before debugging
CoryMartin-NOAA Aug 7, 2023
dc93259
Update gdas_meanioda.yaml
CoryMartin-NOAA Aug 7, 2023
e51dfad
Hopefully will symlink the test YAML
CoryMartin-NOAA Aug 7, 2023
233fb7a
Add some comments
CoryMartin-NOAA Aug 7, 2023
73a661b
A few more hopefully helpful comments
CoryMartin-NOAA Aug 7, 2023
78a1aeb
add working ctest
CoryMartin-NOAA Aug 8, 2023
289352c
Merge branch 'develop' into feature/oops_example
CoryMartin-NOAA Aug 8, 2023
1c01706
Fix things oops
CoryMartin-NOAA Aug 8, 2023
43bdcf9
more oopsies
CoryMartin-NOAA Aug 8, 2023
8bb6a36
ligne vide
CoryMartin-NOAA Aug 8, 2023
34aa28b
force c++17
CoryMartin-NOAA Aug 8, 2023
f899e05
make hera compiler happy
CoryMartin-NOAA Aug 9, 2023
d6a15ae
start of adding cpp linter
CoryMartin-NOAA Aug 9, 2023
47f4303
try to add linter
CoryMartin-NOAA Aug 9, 2023
4fe3246
chmod
CoryMartin-NOAA Aug 9, 2023
04938e7
remove all the legal crap
CoryMartin-NOAA Aug 9, 2023
3af0f76
cpplint
CoryMartin-NOAA Aug 9, 2023
dacb74f
Merge branch 'develop' into feature/cpplint
CoryMartin-NOAA Aug 9, 2023
b75f53b
revert to old include paths
CoryMartin-NOAA Aug 9, 2023
40811fd
make norms part of github action
CoryMartin-NOAA Aug 9, 2023
b75dfad
remove cpplint from pycodestyle
CoryMartin-NOAA Aug 9, 2023
77c0fca
fix things again
CoryMartin-NOAA Aug 9, 2023
1f08ed0
pragma once
CoryMartin-NOAA Aug 10, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions .github/workflows/pynorms.yaml → .github/workflows/norms.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
name: Python Coding Norms
name: Coding Norms
on: [push]

jobs:
check_pynorms:
runs-on: ubuntu-latest
name: Check Python coding norms with pycodestyle
name: Check coding norms with pycodestyle and cpplint

steps:

Expand All @@ -20,4 +20,9 @@ jobs:
- name: Run pycodestyle
run: |
cd $GITHUB_WORKSPACE/GDASApp
pycodestyle -v --config ./.pycodestyle .
pycodestyle -v --config ./.pycodestyle ./ush ./scripts ./test

- name: Run C++ linter on utils
run: |
cd $GITHUB_WORKSPACE/GDASApp/utils/test/
./cpplint.py --quiet --recursive $GITHUB_WORKSPACE/GDASApp/utils
3 changes: 3 additions & 0 deletions utils/CPPLINT.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
set noparent
linelength=100
filter=+build,-legal,+readability,+runtime,+whitespace,-runtime/references,-runtime/printf,-build/include_subdir
25 changes: 17 additions & 8 deletions utils/ioda_example/gdas_meanioda.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#pragma once

#include <iostream>
#include <numeric>
#include <string>
#include <vector>
#include "eckit/config/LocalConfiguration.h"
#include "ioda/Group.h"
#include "ioda/ObsSpace.h"
Expand All @@ -26,11 +30,10 @@ namespace gdasapp {
static const std::string classname() {return "gdasapp::IodaExample";}

int execute(const eckit::Configuration & fullConfig, bool /*validate*/) const {

// get the obs space configuration
const eckit::LocalConfiguration obsConfig(fullConfig, "obs space");
ioda::ObsTopLevelParameters obsparams;
obsparams.validateAndDeserialize(obsConfig); // TODO CRM, can I remove this and then the simulated vars junk??
obsparams.validateAndDeserialize(obsConfig);
oops::Log::info() << "obs space: " << std::endl << obsConfig << std::endl;

// time window stuff
Expand All @@ -51,13 +54,16 @@ namespace gdasapp {

// read the obs space
// Note, the below line does a lot of heavy lifting
// we can probably go to a lower level function (and more of them) to accomplish the same thing
ioda::ObsSpace ospace(obsparams, oops::mpi::world(), util::DateTime(winbegin), util::DateTime(winend), oops::mpi::myself());
// we can probably go to a lower level function
// (and more of them) to accomplish the same thing
ioda::ObsSpace ospace(obsparams, oops::mpi::world(), util::DateTime(winbegin),
util::DateTime(winend), oops::mpi::myself());
const size_t nlocs = ospace.nlocs();
oops::Log::info() << "nlocs =" << nlocs << std::endl;
std::vector<float> buffer(nlocs);

// below is grabbing from the IODA obs space the specified group/variable and putting it into the buffer
// below is grabbing from the IODA obs space
// the specified group/variable and putting it into the buffer
if (chan == 0) {
// no channel is selected
ospace.get_db(group, variable, buffer);
Expand All @@ -67,12 +73,15 @@ namespace gdasapp {
}

// the below line computes the mean, aka sum divided by count
const float mean = std::accumulate(buffer.begin(), buffer.end(), 0) / float(nlocs);
const float mean = std::accumulate(buffer.begin(), buffer.end(), 0) /
static_cast<float>(nlocs);

// write the mean out to the stdout
oops::Log::info() << "mean value for " << group << "/" << variable << "=" << mean << std::endl;
oops::Log::info() << "mean value for " << group << "/" << variable
<< "=" << mean << std::endl;

// a better program should return a real exit code depending on result, but this is just an example!
// a better program should return a real exit code depending on result,
// but this is just an example!
return 0;
}
// -----------------------------------------------------------------------------
Expand Down
12 changes: 8 additions & 4 deletions utils/soca/gdas_incr_handler.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#pragma once

#include <netcdf>
#include <iostream>

#include <filesystem>
#include <iostream>
#include <string>

#include "eckit/config/LocalConfiguration.h"

Expand All @@ -18,7 +22,7 @@
#include "soca/LinearVariableChange/LinearVariableChange.h"
#include "soca/State/State.h"

# include "gdas_postprocincr.h"
#include "gdas_postprocincr.h"

namespace gdasapp {

Expand All @@ -29,7 +33,6 @@ namespace gdasapp {
static const std::string classname() {return "gdasapp::SocaIncrHandler";}

int execute(const eckit::Configuration & fullConfig, bool /*validate*/) const {

/// Setup the soca geometry
const eckit::LocalConfiguration geomConfig(fullConfig, "geometry");
oops::Log::info() << "geometry: " << std::endl << geomConfig << std::endl;
Expand All @@ -39,7 +42,8 @@ namespace gdasapp {
// Initialize the post processing
PostProcIncr postProcIncr(fullConfig, geom, this->getComm());

oops::Log::info() << "soca increments: " << std::endl << postProcIncr.inputIncrConfig_ << std::endl;
oops::Log::info() << "soca increments: " << std::endl
<< postProcIncr.inputIncrConfig_ << std::endl;

// Process list of increments
int result = 0;
Expand Down
33 changes: 16 additions & 17 deletions utils/soca/gdas_postprocincr.h
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#ifndef GDAS_POSTPROCINCR_H
#define GDAS_POSTPROCINCR_H
#pragma once

#include <iostream>
#include <filesystem>

#include <iostream>
#include <string>

#include "eckit/config/LocalConfiguration.h"

#include "atlas/field.h"
Expand All @@ -22,7 +23,7 @@
namespace gdasapp {

class PostProcIncr {
public:
public:
// Constructor
PostProcIncr(const eckit::Configuration & fullConfig, const soca::Geometry& geom,
const eckit::mpi::Comm & comm)
Expand All @@ -33,7 +34,7 @@ class PostProcIncr {
xTraj_(getTraj(fullConfig, geom)),
comm_(comm),
ensSize_(1),
pattern_(){
pattern_() {

oops::Log::info() << "Date: " << std::endl << dt_ << std::endl;

Expand Down Expand Up @@ -73,18 +74,18 @@ class PostProcIncr {
}

// Append layer thicknesses to increment
// TODO: There's got to be a better way to append a variable.
soca::Increment appendLayer(const int n){
// TODO(guillaume): There's got to be a better way to append a variable.
soca::Increment appendLayer(const int n) {
oops::Log::info() << "==========================================" << std::endl;
oops::Log::info() << "====== Append Layers" << std::endl;

// initialize the soca increment
soca::Increment socaIncr(geom_, socaIncrVar_, dt_);
eckit::LocalConfiguration memberConfig; //inputIncrConfig_);
eckit::LocalConfiguration memberConfig; // inputIncrConfig_);
memberConfig = inputIncrConfig_;

// replace templated string if necessary
if (not pattern_.empty()) {
if (!pattern_.empty()) {
util::seekAndReplace(memberConfig, pattern_, std::to_string(n));
oops::Log::info() << "oooooooooooooooooooooooooooooooooooo" << memberConfig << std::endl;
}
Expand Down Expand Up @@ -124,7 +125,7 @@ class PostProcIncr {
// Set specified variables to 0
soca::Increment setToZero(soca::Increment socaIncr) {
oops::Log::info() << "==========================================" << std::endl;
if (not this->setToZero_) {
if (!this->setToZero_) {
oops::Log::info() << "====== no variables to set to 0.0" << std::endl;
return socaIncr;
}
Expand Down Expand Up @@ -154,7 +155,7 @@ class PostProcIncr {
// Apply linear variable changes
soca::Increment applyLinVarChange(soca::Increment socaIncr) {
oops::Log::info() << "==========================================" << std::endl;
if (not this->doLVC_) {
if (!this->doLVC_) {
return socaIncr;
}
oops::Log::info() << "====== applying specified change of variables" << std::endl;
Expand Down Expand Up @@ -252,8 +253,8 @@ class PostProcIncr {
// Utility functions
// -----------------------------------------------------------------------------
// Recreate the soca filename from the configuration
// TODO: Change this in soca?
// TODO: Hard-coded for ocean, implement for seaice as well
// TODO(guillaume): Change this in soca?
// TODO(guillaume): Hard-coded for ocean, implement for seaice as well
std::string socaFname() {
std::string datadir;
outputIncrConfig_.get("datadir", datadir);
Expand Down Expand Up @@ -284,7 +285,7 @@ class PostProcIncr {
}


public:
public:
util::DateTime dt_; // valid date of increment
oops::Variables layerVar_; // layer variable
const soca::Increment Layers_; // layer thicknesses
Expand All @@ -303,6 +304,4 @@ class PostProcIncr {
int ensSize_;
std::string pattern_;
};
} // namespace gdasapp

#endif // GDAS_POSTPROCINCR_H
} // namespace gdasapp
13 changes: 8 additions & 5 deletions utils/soca/gdas_socahybridweights.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#pragma once

#include <netcdf>
#include <iostream>

#include <filesystem>
#include <iostream>
#include <string>

#include "eckit/config/LocalConfiguration.h"

Expand All @@ -13,9 +17,9 @@
#include "oops/util/Duration.h"
#include "oops/util/Logger.h"

#include "soca/State/State.h"
#include "soca/Geometry/Geometry.h"
#include "soca/Increment/Increment.h"
#include "soca/State/State.h"

namespace gdasapp {

Expand All @@ -26,7 +30,6 @@ namespace gdasapp {
static const std::string classname() {return "gdasapp::SocaHybridWeights";}

int execute(const eckit::Configuration & fullConfig, bool /*validate*/) const {

/// Setup the soca geometry
const eckit::LocalConfiguration geomConfig(fullConfig, "geometry");
oops::Log::info() << "geometry: " << std::endl << geomConfig << std::endl;
Expand All @@ -44,7 +47,7 @@ namespace gdasapp {
socaVars += socaOcnVars;

/// Read the background
// TODO: Use the ice extent to set the weights ... no clue if this is
// TODO(guillaume): Use the ice extent to set the weights ... no clue if this is
// possible at this level
soca::State socaBkg(geom, socaVars, dt);
const eckit::LocalConfiguration socaBkgConfig(fullConfig, "background");
Expand All @@ -59,7 +62,7 @@ namespace gdasapp {
oops::Log::info() << "wOcean: " << wOcean << std::endl;

/// Create fields of weights for seaice
soca::Increment socaIceHW(geom, socaVars, dt); // ocean field is mandatory for writting
soca::Increment socaIceHW(geom, socaVars, dt); // ocean field is mandatory for writting
socaIceHW.ones();
socaIceHW *= wIce;
oops::Log::info() << "socaIceHW: " << std::endl << socaIceHW << std::endl;
Expand Down
12 changes: 11 additions & 1 deletion utils/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,18 @@ list( APPEND utils_test_input
file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/testinput)
CREATE_SYMLINK( ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR} ${utils_test_input} )

# copy the cpp linter script
execute_process( COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/cpplint.py ${CMAKE_BINARY_DIR}/bin/${PROJECT_NAME}_cpplint.py)

# add linter for the utils
ecbuild_add_test( TARGET test_gdasapp_util_coding_norms
TYPE SCRIPT
COMMAND ${CMAKE_BINARY_DIR}/bin/${PROJECT_NAME}_cpplint.py
ARGS --quiet --recursive ${CMAKE_CURRENT_SOURCE_DIR}/../
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin )

# Test example IODA utility that computes the mean of a variable
ecbuild_add_test( TARGET test_gdasapp_util_ioda_example
COMMAND ${CMAKE_BINARY_DIR}/bin/gdas_meanioda.x
ARGS "testinput/gdas_meanioda.yaml"
LIBS gdas-utils)
LIBS gdas-utils)
Loading
Loading