Skip to content

Improve design and error checking in decompMod and decompInitMod #3448

@ekluzek

Description

@ekluzek

It turns out there are some design issues and lack of error checking decompMod, and decompInitMod that lead to problems like in #3447. This is where a segmentation fault happens in unrelated places in the code that aren't obvious to track down, because of this. This is an example of brittle code that's hard to work with, that also doesn't have sufficient testing around it, so is harder to work with and takes more time to do so.

Design issues I'm seeing now:

  • decompInitMod allocates and sets module data from decompMod, rather than having decompMod do this.
  • decompMod doesn't have clean allocate/deallocate methods
  • decompMod has many functions that don't do error checking that the structures are even setup
  • decompMod should initialize values before they are setup such that errors will be triggered if used before set
  • decompMod doesn't do error checking for valid values from the decomp setup
  • The subroutines in decompInit are too long, without clear purposes
  • get_proc_bounds is initially used to only return begg/endg, and later for everything, this usage isn't clear from the code...
  • The code in decompInit is ordered, without it being easy/ovious to change the order things are done
  • Code is duplicated between get_proc_bounds and get_proc_clumps (maybe OK now, but with additional error checking, would be good to have the duplicated code in a private subroutine shared by both)
  • Pointers aren't set to null() initially so that you can check they are valid before using them

Error checking:

  • Error checking of namelist items should be done as close to the top as possible
  • Error checking that nsegspc is valid should be done as it can be set by the user
  • Should check that the decomp has been setup, before you can use the functions in decompMod
  • Appropriate range checking of inputs, and the internal arrays and dimension sizes should be done in the decompMod functions

Testing:

  • Adding self tests would be useful to ensure changes are correct and stay correct with development
  • Adding a serial unit tester for the most basic things and error checking testing would be helpful
  • Should add a standard case test to run to the decomp_init test, to make sure a normal test without the special testing framework still works

Sub-issues

Metadata

Metadata

Assignees

Labels

bfbbit-for-bitcode healthimproving internal code structure to make easier to maintain (sustainability)enhancementnew capability or improved behavior of existing capabilitytestingadditions or changes to tests

Type

Projects

Status

Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions