Skip to content

Vs_Profile(add_halfspace=True) leaves material number 0 on the old last layer, so the new profile fails the class's own format check #106

Open

Description

@jsh9

Found while reviewing #74. Pre-existing on main (b764057); #74 keeps the same behavior.

Problem

check_Vs_profile_format() allows material number 0 on the last layer, whatever its thickness, and requires a positive material number on every other layer. So a 5-column profile whose last layer is finite (thickness > 0) and has material number 0 is accepted. With add_halfspace=True, Vs_Profile() appends a half-space row with material number 0 below that layer, but leaves the old last layer at 0. That layer is no longer the last one, so prof.vs_profile breaks the rule above: check_Vs_profile_format() rejects it, and Vs_Profile(prof.vs_profile) raises ValueError.

if add_halfspace and thk[-1] != 0:
last_row = full_data[-1, :]
half_space = last_row.copy()
half_space[0] = 0 # set thickness to 0 meters
half_space[4] = 0 # set material number to 0
full_data = np.vstack((full_data, half_space))
thk, vs, xi, rho, material_number = full_data.T

if np.any(mat[:-1] <= 0):
raise ValueError(
'The "material number" column should be all '
'positive, except for the last error.',
)
if np.any(mat[-1] < 0):
raise ValueError(
'The material number of the last layer should be'
' non-negative.',
)

truncate() already handles the same case: when the layer above the new half-space has material number 0, it gives that layer a new number, max + 1:

# last "material number" before appending is 0
if profile_[-2, -1] == 0:
# require add'l material
profile_[-2, -1] = np.max(profile_[:, -1]) + 1

Also: the error message has a typo. It says "except for the last error" instead of "except for the last layer" (line 540 above). The repro shows the message.

import numpy as np
import PySeismoSoil.helper_generic as hlp
from PySeismoSoil.class_Vs_profile import Vs_Profile

data = np.array([
    [10.0, 200.0, 0.05, 1800.0, 1.0],
    [20.0, 300.0, 0.05, 1800.0, 0.0],  # last layer: finite, material number 0
])
hlp.check_Vs_profile_format(data)  # accepted

prof = Vs_Profile(data, add_halfspace=True)
print(prof.vs_profile[:, [0, 4]])
# [[10.  1.]
#  [20.  0.]
#  [ 0.  0.]]
# (expected [[10.  1.]
#            [20.  2.]
#            [ 0.  0.]])

try:
    hlp.check_Vs_profile_format(prof.vs_profile)
except ValueError as e:
    print(type(e).__name__, e)
# ValueError The "material number" column should be all positive, except for the last error.
# (expected: no error)

try:
    Vs_Profile(prof.vs_profile)
except ValueError as e:
    print(type(e).__name__, e)
# ValueError The "material number" column should be all positive, except for the last error.
# (expected: no error)

# truncate() handles the same case by giving the layer a new material number
print(Vs_Profile(data).truncate(depth=40).vs_profile[:, [0, 4]])
# [[10.  1.]
#  [30.  2.]
#  [ 0.  0.]]

(truncate() also changes the original profile in place, see #75. That does not affect the output above.)

The existing tests test_add_halfspace__case_1__already_a_half_space and test_add_halfspace__case_1__no_half_space on main (in #74: test_add_halfspace[already_a_half_space] and test_add_halfspace[no_half_space]) do not cover this case. The first file already has a half-space, and the second is a 2-column profile, which gets material numbers 1..n.

Fix

  1. In Vs_Profile.__init__, before appending the half-space, give the old last layer a new material number when it has 0, as truncate() does:

                half_space[4] = 0  # set material number to 0
    +            if full_data[-1, 4] == 0:  # the old last layer needs a material
    +                full_data[-1, 4] = np.max(full_data[:, 4]) + 1
    +
                full_data = np.vstack((full_data, half_space))
  2. In check_Vs_profile_format(), change "except for the last error." to "except for the last layer.". The material_number_not_positive case of test_Vs_profile_format (in Combine near-duplicate test functions into parametrized tests #74; test_Vs_profile_format__case_10 on main) matches only 'should be all positive', so it still passes.

  3. Add a case to test_add_halfspace for a 5-column profile without a half-space whose last layer has material number 0. Check that the material numbers become [1, 2, 0] and that Vs_Profile(prof.vs_profile) works.

With only steps 1 and 2 on main, the repro gives the expected output, and all 240 tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions