lballabio / QuantLib

The QuantLib C++ library
http://quantlib.org
Other
5.26k stars 1.78k forks source link

Memory access violation on latest MSCV toolset in GitHub Actions runners #1987

Closed lballabio closed 1 month ago

lballabio commented 3 months ago

The latest Windows runner images switched the MSVC toolset from 14.3 to 14.4.

When using the dynamic runtime ("Release" configuration in the VC++ project) this results in the test suite crashing during the Qd+ American engine tests; see e.g. https://github.com/lballabio/QuantLib/actions/runs/9439432945/job/25997550707.

lballabio commented 3 months ago

@klausspanderen any chances you can find some time to look into this? Thanks!

ralfkonrad commented 3 months ago

Hi @lballabio,

have you tried to call the ilammy/msvc-dev-cmd action without specifying the toolset and the vsversion? According to the actions inputs they are not necessary.

Also, wouldn't it be toolset = 14.40 rather than 14.4 according to here? In the workflow output you can see that the action is struggling to find the right combination.

lballabio commented 3 months ago

I did try without toolset, same result. I didn't try removing vsversion, I'll do it next.

ralfkonrad commented 3 months ago

Okay, but I guess vsversion will not change anything here as 2022 is correct anyhow.

lballabio commented 3 months ago

Indeed—the error is the same, see https://github.com/lballabio/QuantLib/actions/runs/9445985967/job/26014696954

klausspanderen commented 3 months ago

Hi, I have a look but unfortunately I can't reproduce it yet locally on my windows machine with toolset = 14.40. In addition valgrind does not show any memory error on a linux machine. It may take longer...

ralfkonrad commented 3 months ago

I also can't reproduce it on my machine. I replayed the workflow file from the test branch

> # unzip boost into C:\local\boost
> COPY .ci\VS2022.props .\Build.props
> COPY .ci\Unity.props .\Directory.Build.props   # and also skipping the Directory.Build.props based build
> msbuild ./QuantLib.sln /verbosity:normal /property:Configuration=Release /property:Platform=x64
...
> .\test-suite\bin\QuantLib-test-suite*.exe --log_level=message
...

and the test suite passes without errors.

There are only two differences in the two approaches:

> gci env:* | sort-object name

Name                           Value
----                           -----
...
INCLUDE                        C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\include;C:\Program Files\Microsoft Visual Studio\2022\Profes...
...
LIB                            C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\ATLMFC\lib\x64;C:\Program Files\Microsoft Visual Studio\2022...
LIBPATH                        C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\ATLMFC\lib\x64;C:\Program Files\Microsoft Visual Studio\2022...
...
VCIDEInstallDir                C:\Program Files\Microsoft Visual Studio\2022\Professional\Common7\IDE\VC\
VCINSTALLDIR                   C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\
VCToolsInstallDir              C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\
VCToolsRedistDir               C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Redist\MSVC\14.40.33807\
VCToolsVersion                 14.40.33807
...
klausspanderen commented 3 months ago

Thanks Ralf for trying it out. I still can't reproduce it locally but I get the github workflow to fail with this minimal test case

BOOST_AUTO_TEST_CASE(testQdAmericanEngines) {
    BOOST_TEST_MESSAGE("Testing QD+ American option pricing...");

    const DayCounter dc = Actual365Fixed();
    const Date today = Date(1, June, 2022);
    Settings::instance().evaluationDate() = today;

    const auto spot = ext::make_shared<SimpleQuote>(87.54302345597453);
    const auto rRate = ext::make_shared<SimpleQuote>(0.1661983527161978);
    const auto qRate = ext::make_shared<SimpleQuote>(0.1083892627983468);
    const auto vol = ext::make_shared<SimpleQuote>(1.95177229014707);

    const auto bsProcess = ext::make_shared<BlackScholesMertonProcess>(
        Handle<Quote>(spot),
        Handle<YieldTermStructure>(flatRate(today, qRate, dc)),
        Handle<YieldTermStructure>(flatRate(today, rRate, dc)),
        Handle<BlackVolTermStructure>(flatVol(today, vol, dc))
    );

    const auto qrPlusAmericanEngine =
        ext::make_shared<QdPlusAmericanEngine>(
            bsProcess, 8, QdPlusAmericanEngine::Halley, 1e-10
        );

    const Date maturityDate = today + Period(9, Days);

    VanillaOption option(
        ext::make_shared<PlainVanillaPayoff>(
            Option::Call, 261.4820526297886),
        ext::make_shared<AmericanExercise>(today, maturityDate)
    );
    option.setPricingEngine(qrPlusAmericanEngine);

    const Real calculated = option.NPV();
}

The memory access violation occurs during the boost tanh_sinh integration (qlplusamericanengine.cpp, line 376) . Everything works fine if I use QL's GaussLobattoIntegration instead but the error might be in a different spot.

lballabio commented 3 months ago

Thanks, @klausspanderen - that's further than I was able to go. Should we change integration on master to fix the builds while investigating further?

lballabio commented 3 months ago

On hold for the moment since the latest images do work.

ralfkonrad commented 3 months ago

There are discussions about a Broken C++ runtime on windows-2022 version 20240603.1.0.

So perhaps it is not really related to our code.

klausspanderen commented 1 month ago

shall we close this one?

lballabio commented 1 month ago

Yes, let's.