einola commented 6 months ago

A fix to the crash reported in issue 470

Fixes #470

Task List

[ ] Defined the tests that specify a complete and functioning change (It may help to create a design specification & test specification)
[ ] Implemented the source code change that satisfies the tests
[ ] Documented the feature by providing worked example
[ ] Updated the README or other documentation
[ ] Completed the pre-Request checklist below

Change Description

The function Logged::configure() is trying to access keyMap, but this is not accessible. I propose fixing this by adding keyMap to Logged.hpp in the protected clause. I'm not sure if this is the right place or if it should be public or private. But the model runs now (at least the thermo test).

Test Description

Run the thermodynamics integration test (demo), i.e. nextsim --config-file config_column.cfg

Before the fix the model crashed with a sigsev message (Process finished with exit code 139 (interrupted by signal 11: SIGSEGV)). After the fix, the model runs through and produces the expected output: a netCDF file called diagnostic.nc, which produces the right plots when column_pp.py is run.

Documentation Impact

N/A

Other Details

N/A

Pre-Request Checklist

[ ] The requirements of this pull request are fully captured in an issue or design specification and are linked and summarised in the description of this PR
[ ] No new warnings are generated
[ ] The documentation has been updated (or an issue has been created to track the corresponding change)
[ ] Methods and Tests are commented such that they can be understood without having to obtain additional context
[ ] This PR/Issue is labelled as a bug/feature/enhancement/breaking change
[ ] File dates have been updated to reflect modification date
[ ] This change conforms to the conventions described in the README

timspainNERSC commented 6 months ago

The changes here look good, but I think the Logged class should inherit from Configured. Let me check if that also solves the problem. If it does, then we should use that solution instead.

timspainNERSC commented 6 months ago

Also, the changes look good, but don't fix the SEGFAULT for me.

timspainNERSC commented 6 months ago

Superseded by PR #472

nextsimhub / nextsimdg