LLNL / hiop

HPC solver for nonlinear optimization problems
Other
210 stars 42 forks source link

Narrowing conversion in Ginko #655

Closed nychiang closed 1 year ago

nychiang commented 1 year ago

Using C++17 and compiler flags "-Wall -Werror, I got the following error on PNNL CI:

/src/LinAlg/hiopLinSolverSparseGinkgo.cpp:178:89: error: narrowing conversion of 'n_' from 'int' to 'gko::dim<2>::dimension_type' {aka 'long unsigned int'} inside { } [-Werror=narrowing]
     auto mtx = gko::share(gko::matrix::Csr<double, int>::create(exec, gko::dim<2>{n_, n_}, val_array, col_idxs, row_ptrs));

It seems to me Ginkgo use long unsigned int inside function gko::dim.

For more details, see #653 #654

@pelesh @tepperly @cnpetra

pelesh commented 1 year ago

Does explicitly casting n_ to unsigned integer before passing it to Ginkgo resolves the issue?

nychiang commented 1 year ago

Yes, manually casting those variables work fine. Now everything works on Newell. However, on Marianas, I got the following error from different places :

people/svcexasgd/gitlab/126079/src/LinAlg/hiopLinSolverSparseGinkgo.cpp:175:42: error: 'using Array = class gko::array<double>' is deprecated: please use array [-Werror=deprecated-declarations]
  175 |     auto val_array = gko::Array<double>::view(exec, nnz_, kVal_);
      |                                          ^~~~
In file included from /qfs/projects/exasgd/src/ci-deception/linux-centos7-zen2/gcc-10.2.0/ginkgo-1.5.0.glu_experimental-3o5dw4rvskf3bkknmfzth252ybr4crcf/include/ginkgo/ginkgo.hpp:41,
                 from /people/svcexasgd/gitlab/126079/src/LinAlg/hiopLinSolverSparseGinkgo.hpp:55,
                 from /people/svcexasgd/gitlab/126079/src/LinAlg/hiopLinSolverSparseGinkgo.cpp:55:
/qfs/projects/exasgd/src/ci-deception/linux-centos7-zen2/gcc-10.2.0/ginkgo-1.5.0.glu_experimental-3o5dw4rvskf3bkknmfzth252ybr4crcf/include/ginkgo/core/base/array.hpp:717:7: note: declared here
  717 | using Array [[deprecated("please use array")]] = array<ValueType>;

Any suggestion?

@fritzgoebel @pelesh @cnpetra

cnpetra commented 1 year ago

maybe avoid deprecated call?

fritzgoebel commented 1 year ago

I am working on updating the Ginkgo interface and will get rid of the deprecated call in the process.

nychiang commented 1 year ago

@fritzgoebel Thanks for taking care of this. In fact, I tried to fix it in PR #654. Right now it passes all the designed pipeline tests. Please have a look of it, as it is ready for merge.

@cnpetra

fritzgoebel commented 1 year ago

Sorry, I was not aware of the PR. Looks good to me.

nychiang commented 1 year ago

Closed by PR #654 @cnpetra