-
Notifications
You must be signed in to change notification settings - Fork 95
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
Release V4.4.0 #2383
Release V4.4.0 #2383
Conversation
I don't know of anything else that needs to go in. CMake stuff would be nice, but not essential for me at the moment - happy to leave to 4.4.1 or even (if it's going to be much work) 5.0. |
Would it make sense to do a v4.3.3 release as well? There has been a surprisingly large number of commits since v4.3.2 to master: If its to much effort, we could preserve current master as v4.3.x branch |
Thanks, a v4.3.3 release is a good idea! There's more commits than I thought. That release shouldn't be too hard, I'll get it started now. This branch needs:
Plus the following tests are failing:
|
|
This on the first check:
Adding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 25 out of 182. Check the log or trigger a new build to see more.
As far as I remember, when I was writing this test originally, I tried to use |
It looks like if I set |
Yes, I think |
Test checks that FieldGroup(int) doesn't compile: this can now be done via std::is_constructable
_very_ difficult to reproduce, probably rounding error
This is consistent with the existing Makefile approach and removes the need for EXECUTABLE_NAME
Allow PETSc options to be passed from BOUT.inp (v4.4)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 25 out of 208. Check the log or trigger a new build to see more.
Fix some HDF5 related issues and add Mesh::getLocal{X,Y,Z}Index (v4.4)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 25 out of 189. Check the log or trigger a new build to see more.
clang-tidy does not like the uuid header. Wish I'd added it to an ignore list! |
Necessary to add FieldPerp to variant in Options
Needed to add FieldPerp to Options
Move implementation from DataFormat methods to FieldPerp
Allows storing `FieldPerp` in `Options`
Also check that y-indices match in operator==
`FieldPerp::getGlobalIndex` calls `Mesh::hasBndrylowery` which calls `MPI_Allreduce` and `Mesh::getXcomm`. These fixes were already in `next` when the `OptionsNetCDF` test with `FieldPerp` was added there
Adding an adaptive, arbitrary order, Adams-Bashforth solver (v4.4)
Add Mesh::getRegion<T> for use in generic code (v4.4)
Add ability to use FieldPerp in Options (v.4.4)
This is now ready for release! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 25 out of 179. Check the log or trigger a new build to see more.
if (std::is_same<CharT, char>::value) | ||
size = strlen(name); | ||
else | ||
size = wcslen(name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: statement should be inside braces [readability-braces-around-statements]
else
^
{
void reset() { | ||
hasher.reset(); | ||
char bytes[16]; | ||
auto nsbytes = nsuuid.as_bytes(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]
char bytes[16];
^
hasher.reset(); | ||
char bytes[16]; | ||
auto nsbytes = nsuuid.as_bytes(); | ||
strncpy(bytes, nsbytes, 16); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: auto nsbytes
can be declared as const auto *nsbytes
[readability-qualified-auto]
auto nsbytes = nsuuid.as_bytes();
^~~~~
const auto *
void process_characters(char_type const* const characters, size_t const count) { | ||
for (size_t i = 0; i < count; i++) { | ||
uint32_t c = characters[i]; | ||
hasher.process_byte(static_cast<unsigned char>((c >> 0) & 0xFF)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]
uint32_t c = characters[i];
^
digest[6] |= 0x50; | ||
|
||
return uuid{digest, digest + 16}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]
return uuid{digest, digest + 16};
^
|
||
const char *hdf5_match[] = {"h5","hdf","hdf5"}; | ||
for(int i=0; i<3; i++) { | ||
if(strcasecmp(s, hdf5_match[i]) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
if(strcasecmp(s, hdf5_match[i]) == 0) {
^
@@ -73,8 +73,8 @@ inline const DEPRECATED(Field3D Grad_par(const Field3D& var, CELL_LOC outloc, | |||
DIFF_METHOD method)) { | |||
return Grad_par(var, outloc, toString(method)); | |||
}; | |||
DEPRECATED(inline const DEPRECATED( | |||
Field3D Grad_par(const Field3D& var, DIFF_METHOD method, CELL_LOC outloc))) { | |||
DEPRECATED(inline const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: return type const Field3D
is const
-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type]
DEPRECATED(inline const
^
/github/workspace/include/bout/deprecated.hxx:18:26: note: expanded from macro 'DEPRECATED'
#define DEPRECATED(func) __attribute__ ((deprecated)) func
^
@@ -87,6 +90,8 @@ void DST_rev(dcomplex *in, int length, BoutReal *out); | |||
/// Should the FFT functions find and use an optimised plan? | |||
void fft_init(bool fft_measure); | |||
/// Should the FFT functions find and use an optimised plan? | |||
void fft_init(FFT_MEASUREMENT_FLAG fft_flag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function bout::fft::fft_init
has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
void fft_init(FFT_MEASUREMENT_FLAG fft_flag);
^
/github/workspace/src/invert/fft_fftw.cxx:94:6: note: the definition seen here
void fft_init(FFT_MEASUREMENT_FLAG fft_measurement_flag) {
^
include/fft.hxx:93:6: note: differing parameters are named here: ('fft_flag'), in definition: ('fft_measurement_flag')
void fft_init(FFT_MEASUREMENT_FLAG fft_flag);
^ ~~~~~~~~
fft_measurement_flag
|
||
protected: | ||
CELL_LOC location; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: member variable location
has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]
CELL_LOC location;
^
"const std::string& region = \"RGN_ALL\") instead")]] | ||
inline const Field2D abs(const Vector2D &v, REGION region) { | ||
return abs(v, toString(region)); | ||
} | ||
|
||
/// Transform to and from field-aligned coordinates | ||
inline Vector2D toFieldAligned(Vector2D v, const std::string& UNUSED(region) = "RGN_ALL") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: the parameter v
is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
inline Vector2D toFieldAligned(Vector2D v, const std::string& UNUSED(region) = "RGN_ALL") {
^
const &
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rubber stamping... Hopefully we reviewed all the PRs, so there's no need to review these changes carefully!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 25 out of 150. Check the log or trigger a new build to see more.
/*! | ||
* Constructor from Array and Mesh | ||
*/ | ||
FieldPerp(Array<BoutReal> data, Mesh* fieldmesh, CELL_LOC location_in = CELL_CENTRE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function FieldPerp::FieldPerp
has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
FieldPerp(Array<BoutReal> data, Mesh* fieldmesh, CELL_LOC location_in = CELL_CENTRE,
^
/github/workspace/src/field/fieldperp.cxx:51:12: note: the definition seen here
FieldPerp::FieldPerp(Array<BoutReal> data_in, Mesh* localmesh, CELL_LOC location_in,
^
include/fieldperp.hxx:84:3: note: differing parameters are named here: ('fieldmesh'), in definition: ('localmesh')
FieldPerp(Array<BoutReal> data, Mesh* fieldmesh, CELL_LOC location_in = CELL_CENTRE,
^ ~~~~~~~~~
localmesh
@@ -668,6 +697,7 @@ template<> inline void Options::assign<>(const char *val, const std::string sour | |||
// Note: Field assignments don't check for previous assignment (always force) | |||
template<> void Options::assign<>(Field2D val, const std::string source); | |||
template<> void Options::assign<>(Field3D val, const std::string source); | |||
template<> void Options::assign<>(FieldPerp val, const std::string source); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: parameter source
is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]
template<> void Options::assign<>(FieldPerp val, const std::string source);
^~~~~~
@@ -724,4 +755,11 @@ template <> Field3D Options::as<Field3D>(const Field3D& similar_to) const; | |||
Options::getRoot()->getSection("all")->get(#var, var, def); \ | |||
}} \ | |||
|
|||
/// Define for over-riding library defaults for options, should be called in global | |||
/// namespace so that the new default is set before main() is called. | |||
#define BOUT_OVERRIDE_DEFAULT_OPTION(name, value) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function-like macro BOUT_OVERRIDE_DEFAULT_OPTION
used; consider a constexpr
template function [cppcoreguidelines-macro-usage]
#define BOUT_OVERRIDE_DEFAULT_OPTION(name, value) \
^
// of the argument (hence pass-by-value instead of pass-by-reference) | ||
return v; | ||
} | ||
inline Vector2D fromFieldAligned(Vector2D v, const std::string& UNUSED(region) = "RGN_ALL") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: the parameter v
is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
inline Vector2D fromFieldAligned(Vector2D v, const std::string& UNUSED(region) = "RGN_ALL") {
^
const &
@@ -118,6 +118,27 @@ const Vector3D Grad_perp(const Field3D &f, CELL_LOC outloc, const std::string& m | |||
return result; | |||
} | |||
|
|||
const Vector2D Grad_perp(const Field2D &f, CELL_LOC outloc, const std::string& method) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: return type const Vector2D
is const
-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type]
const Vector2D Grad_perp(const Field2D &f, CELL_LOC outloc, const std::string& method) {
^~~~~~
@@ -97,12 +99,16 @@ class Ncxx4 : public DataFormat { | |||
|
|||
bool read(int *var, const char *name, int lx = 1, int ly = 0, int lz = 0) override; | |||
bool read(int *var, const std::string &name, int lx = 1, int ly = 0, int lz = 0) override; | |||
bool read(char *var, const char *name, int n = 1) override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function Ncxx4::read
has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
bool read(char *var, const char *name, int n = 1) override;
^
/github/workspace/src/fileio/impls/netcdf4/ncxx4.cxx:591:13: note: the definition seen here
bool Ncxx4::read(char *data, const char *name, int n) {
^
src/fileio/impls/netcdf4/ncxx4.hxx:102:8: note: differing parameters are named here: ('var'), in definition: ('data')
bool read(char *var, const char *name, int n = 1) override;
^ ~~~
data
bool read(BoutReal *var, const char *name, int lx = 1, int ly = 0, int lz = 0) override; | ||
bool read(BoutReal *var, const std::string &name, int lx = 1, int ly = 0, int lz = 0) override; | ||
bool read_perp(BoutReal *var, const std::string &name, int lx = 1, int lz = 0) override; | ||
|
||
bool write(int *var, const char *name, int lx = 0, int ly = 0, int lz = 0) override; | ||
bool write(int *var, const std::string &name, int lx = 0, int ly = 0, int lz = 0) override; | ||
bool write(char *var, const char *name, int n = 1) override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function Ncxx4::write
has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
bool write(char *var, const char *name, int n = 1) override;
^
/github/workspace/src/fileio/impls/netcdf4/ncxx4.cxx:726:13: note: the definition seen here
bool Ncxx4::write(char *data, const char *name, int n) {
^
src/fileio/impls/netcdf4/ncxx4.hxx:110:8: note: differing parameters are named here: ('var'), in definition: ('data')
bool write(char *var, const char *name, int n = 1) override;
^ ~~~
data
@@ -111,12 +117,16 @@ class Ncxx4 : public DataFormat { | |||
|
|||
bool read_rec(int *var, const char *name, int lx = 1, int ly = 0, int lz = 0) override; | |||
bool read_rec(int *var, const std::string &name, int lx = 1, int ly = 0, int lz = 0) override; | |||
bool read_rec(char *var, const char *name, int n = 1) override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function Ncxx4::read_rec
has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
bool read_rec(char *var, const char *name, int n = 1) override;
^
/github/workspace/src/fileio/impls/netcdf4/ncxx4.cxx:899:13: note: the definition seen here
bool Ncxx4::read_rec(char *data, const char *name, int n) {
^
src/fileio/impls/netcdf4/ncxx4.hxx:120:8: note: differing parameters are named here: ('var'), in definition: ('data')
bool read_rec(char *var, const char *name, int n = 1) override;
^ ~~~
data
bool read_rec(BoutReal *var, const char *name, int lx = 1, int ly = 0, int lz = 0) override; | ||
bool read_rec(BoutReal *var, const std::string &name, int lx = 1, int ly = 0, int lz = 0) override; | ||
bool read_rec_perp(BoutReal *var, const std::string &name, int lx = 1, int lz = 0) override; | ||
|
||
bool write_rec(int *var, const char *name, int lx = 0, int ly = 0, int lz = 0) override; | ||
bool write_rec(int *var, const std::string &name, int lx = 0, int ly = 0, int lz = 0) override; | ||
bool write_rec(char *var, const char *name, int n = 1) override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function Ncxx4::write_rec
has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
bool write_rec(char *var, const char *name, int n = 1) override;
^
/github/workspace/src/fileio/impls/netcdf4/ncxx4.cxx:1033:13: note: the definition seen here
bool Ncxx4::write_rec(char *data, const char *name, int n) {
^
src/fileio/impls/netcdf4/ncxx4.hxx:128:8: note: differing parameters are named here: ('var'), in definition: ('data')
bool write_rec(char *var, const char *name, int n = 1) override;
^ ~~~
data
: localmesh(m==nullptr ? bout::globals::mesh : m), location(loc) { | ||
int LaplaceXY::instance_count = 0; | ||
|
||
LaplaceXY::LaplaceXY(Mesh* m, Options* opt, const CELL_LOC loc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: constructor does not initialize these fields: MatA, xs, bs, ksp, pc, x_inner_dirichlet, x_outer_dirichlet [cppcoreguidelines-pro-type-member-init]
LaplaceXY::LaplaceXY(Mesh* m, Options* opt, const CELL_LOC loc)
^
Thanks @johnomotani ! I'm not sure why the tests failed, looks like a Github error. |
This is long overdue. There's probably still some things we want to backport from
next
into this. Probably getting a bunch of the CMake stuff in is a good idea. @johnomotani Anything you think is missing?Making a New Release of BOUT++
This is checklist of things to do (in order) when making a new
release. This applies equally to both major/minor releases and bugfix
releases
vX.Y.Z-rc
(actually just using thev4.4.0-alpha
branch)X
/Y
) should be offnext
. Bugfix releases (Z
) should be offmaster
master
"bugfixes" can include:
make check-all
clang-tidy
,clang-check
,coverity
, etc.Before merging PR:
make -C locale update-all
reorder file paths in the .po and .pot files
CHANGELOG.md
][changelog]:github_changelog_generator
make changelog LAST_VERSION=vA.B.C RELEASE_BRANCH=master|next
RELEASE_BRANCH
might need to be the RC branch to getbugfix PRs
CHANGELOG.md
][changelog]!\(\)
git log --format='%aN' | sort | uniq
CITATION.cff
, add new authorsCITATION.cff
to new DOICITATION.cff
abidiff
to see ifsoname
needs bumping inmakefile
:configure.ac
:AC_INIT
CITATION.cff
:version
manual/sphinx/conf.py
:version
andrelease
manual/doxygen/Doxyfile_readthedocs
:PROJECT_NUMBER
manual/doxygen/Doxyfile
:PROJECT_NUMBER
After PR is merged:
make dist
leading
v
master
intonext