search

OPM / opm-models

The models module for the Open Porous Media Simulation (OPM) framework
Other
17 stars 69 forks source link

Feature/add failure flag to tasklets #909

Closed lisajulia closed 1 month ago

lisajulia commented 2 months ago

As suggested by https://github.com/OPM/opm-common/pull/3870#issuecomment-1911661089, adding a failureFlag to the TaskletRunner, will solve https://github.com/OPM/opm-models/issues/871

lisajulia commented 2 months ago

jenkins build this please

lisajulia commented 2 months ago

jenkins build this please

lisajulia commented 2 months ago

jenkins build this please

blattms commented 2 months ago

Interesting failure for python_basic:

Error: [.../opm-common/opm/io/eclipse/EclFile.cpp:245] Could not open file: './SPE1CASE1.UNRST'
ERROR: Uncaught std::exception when running tasklet: [.../opm-common/opm/io/eclipse/EclFile.cpp:245] Could not open file: './SPE1CASE1.UNRST'.

Probably this appeared previously and was not noticed. Question is whether this is a fatal failure or something that should be ignored.

Once that is sorted out, I'll merge.

lisajulia commented 2 months ago

Interesting failure for python_basic:

Error: [.../opm-common/opm/io/eclipse/EclFile.cpp:245] Could not open file: './SPE1CASE1.UNRST'
ERROR: Uncaught std::exception when running tasklet: [.../opm-common/opm/io/eclipse/EclFile.cpp:245] Could not open file: './SPE1CASE1.UNRST'.

Probably this appeared previously and was not noticed. Question is whether this is a fatal failure or something that should be ignored.

Once that is sorted out, I'll merge.

Yes! Indeed! I'll look into that!

lisajulia commented 2 months ago

jenkins build this opm-simulators=5474 please

lisajulia commented 2 months ago

Interesting failure for python_basic:

Error: [.../opm-common/opm/io/eclipse/EclFile.cpp:245] Could not open file: './SPE1CASE1.UNRST'
ERROR: Uncaught std::exception when running tasklet: [.../opm-common/opm/io/eclipse/EclFile.cpp:245] Could not open file: './SPE1CASE1.UNRST'.

Probably this appeared previously and was not noticed. Question is whether this is a fatal failure or something that should be ignored.

Once that is sorted out, I'll merge.

That is very strange, the case SPE1CASE1.DATA runs through when I call flow without python, but the test failed. Anyhow: removing the keyword UNIFOUT solved it, now the python test runs through as well.

lisajulia commented 1 month ago

@bska and @blattms: Can you have a look a this? Thanks! :)

lisajulia commented 1 month ago

jenkins build this please

lisajulia commented 1 month ago

@bska: Can you have another look here?

lisajulia commented 1 month ago

I might personally consider using one of the STL lock guards like std::unique_lock or std::lock_guard instead of directly calling the std::mutex::lock() and std::mutex::unlock() member functions, but in such simple test code it's probably a wash. On the other hand, I don't quite understand why we're using an owning raw pointer for the TaskletRunner object in the new unit test. Would you care to elaborate on the reason for this?

Thanks, I've added comments here: https://github.com/OPM/opm-models/pull/909/commits/cedd34a379c211ab8a2e45577436e3e62e796e10, is that ok now? Otherwise I can also remove the assertion and outputs.

lisajulia commented 1 month ago 
    runner = std::make_unique<Opm::TaskletRunner>(numWorkers);

Yes, thanks! I've changed that now in https://github.com/OPM/opm-models/pull/909/commits/f1b4182ef636cfd19242da9227613ae2a995e538

lisajulia commented 1 month ago

jenkins build this please