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

FastSmallVector: initialize array member #4458

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akva2
Copy link
Member

@akva2 akva2 commented Feb 3, 2025

Quells downstream warning

In file included from /home/akva/kode/opm/opm-common/opm/material/densead/EvaluationSpecializations.hpp:34,
                 from /home/akva/kode/opm/opm-common/opm/material/densead/Evaluation.hpp:631,
                 from /home/akva/kode/opm/opm-simulators/opm/simulators/aquifers/AquiferAnalytical.hpp:32,
                 from /home/akva/kode/opm/opm-simulators/opm/simulators/aquifers/AquiferCarterTracy.hpp:30,
                 from /home/akva/kode/opm/opm-simulators/opm/simulators/aquifers/BlackoilAquiferModel.hpp:32,
                 from /home/akva/kode/opm/opm-simulators/opm/simulators/flow/SimulatorFullyImplicitBlackoil.hpp:43,
                 from /home/akva/kode/opm/opm-simulators/flow/flow_oilwater_polymer.cpp:25:
In member function ‘Opm::DenseAd::Evaluation<ValueT, -1, staticSize>& Opm::DenseAd::Evaluation<ValueT, -1, staticSize>::operator-=(const RhsValueType&) [with RhsValueType = double; ValueT = double; unsigned int staticSize = 7]’,
    inlined from ‘Opm::DenseAd::Evaluation<ValueT, -1, staticSize> Opm::DenseAd::Evaluation<ValueT, -1, staticSize>::operator-(const RhsValueType&) const [with RhsValueType = double; ValueT = double; unsigned int staticSize = 7]’ at /home/akva/kode/opm/opm-common/opm/material/densead/DynamicEvaluation.hpp:382:16,
    inlined from ‘Evaluation Opm::Tabulated1DFunction<Scalar>::eval(const Evaluation&, Opm::SegmentIndex) const [with Evaluation = Opm::DenseAd::Evaluation<double, -1, 7>; Scalar = double]’ at /home/akva/kode/opm/opm-common/opm/material/common/Tabulated1DFunction.hpp:278:34,
    inlined from ‘Evaluation Opm::Tabulated1DFunction<Scalar>::eval(const Evaluation&, bool) const [with Evaluation = Opm::DenseAd::Evaluation<double, -1, 7>; Scalar = double]’ at /home/akva/kode/opm/opm-common/opm/material/common/Tabulated1DFunction.hpp:265:30,
    inlined from ‘Opm::BlackOilPolymerModule<Opm::Properties::TTag::FlowOilWaterPolymerProblem, true>::computeShearFactor<Opm::DenseAd::Evaluation<double, -1, 7> >(const Opm::DenseAd::Evaluation<double, -1, 7>&, unsigned int, const Opm::DenseAd::Evaluation<double, -1, 7>&)::<lambda(const Opm::DenseAd::Evaluation<double, -1, 7>&)>’ at /home/akva/kode/opm/opm-simulators/opm/models/blackoil/blackoilpolymermodules.hh:499:53,
    inlined from ‘static Evaluation Opm::BlackOilPolymerModule<TypeTag, enablePolymerV>::computeShearFactor(const Evaluation&, unsigned int, const Evaluation&) [with Evaluation = Opm::DenseAd::Evaluation<double, -1, 7>; TypeTag = Opm::Properties::TTag::FlowOilWaterPolymerProblem; bool enablePolymerV = true]’ at /home/akva/kode/opm/opm-simulators/opm/models/blackoil/blackoilpolymermodules.hh:512:18:
/home/akva/kode/opm/opm-common/opm/material/densead/DynamicEvaluation.hpp:274:28: warning: ‘<unnamed>.Opm::DenseAd::Evaluation<double, -1, 7>::data_.Opm::FastSmallVector<double, 7>::smallBuf_.std::array<double, 7>::_M_elems[0]’ may be used uninitialized [-Wmaybe-uninitialized]
  274 |         data_[valuepos_()] -= other;
      |         ~~~~~~~~~~~~~~~~~~~^~~~~~~~

@akva2
Copy link
Member Author

akva2 commented Feb 3, 2025

jenkins build this please

@bska
Copy link
Member

bska commented Feb 3, 2025

We may want to benchmark this. From a C++ point of view this is obviously fine, but the FastSmallVector is supposed to be fast (and small). This forced initialisation for built-in scalar types such as double may be a bit too costly.

@akva2
Copy link
Member Author

akva2 commented Feb 3, 2025

I agree but I think you'd be hard pressed to pick it up in anything but micro benches and I don't think you'll see it in the macro benches. But let's run the benches at least.

@akva2
Copy link
Member Author

akva2 commented Feb 3, 2025

benchmark please

@ytelses
Copy link

ytelses commented Feb 5, 2025

Benchmark result overview:

Test Configuration Relative
opm-git OPM Benchmark: drogon - Threads: 1 0.997
opm-git OPM Benchmark: drogon - Threads: 8 1.211
opm-git OPM Benchmark: punqs3 - Threads: 1 0.981
opm-git OPM Benchmark: punqs3 - Threads: 8 1.009
opm-git OPM Benchmark: smeaheia - Threads: 1 0.994
opm-git OPM Benchmark: smeaheia - Threads: 8 1.079
opm-git OPM Benchmark: spe10_model_1 - Threads: 1 1.002
opm-git OPM Benchmark: spe10_model_1 - Threads: 8 0.99
opm-git OPM Benchmark: flow_mpi_extra - Threads: 1 1
opm-git OPM Benchmark: flow_mpi_extra - Threads: 8 1.129
opm-git OPM Benchmark: flow_mpi_norne - Threads: 1 0.996
opm-git OPM Benchmark: flow_mpi_norne - Threads: 8 1.076
opm-git OPM Benchmark: flow_mpi_norne_4c_msw - Threads: 1 - FOPT (Total Oil Production At End Of Run) 1
opm-git OPM Benchmark: flow_mpi_norne_4c_msw - Threads: 8 - FOPT (Total Oil Production At End Of Run) 1
  • Speed-up = Total time master / Total time pull request. Above 1.0 is an improvement. *

View result details @ https://www.ytelses.com/opm/?page=result&id=2693

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants