Repository navigation
The lenient docstrings of Site_Factors and Site_Effect_Adjustment describe the opposite of what the code does #91
Description
Found while reviewing #74. Pre-existing on main (b764057); #74 does not change the behavior.
Problem
Both docstrings say that out-of-range Vs30, z1 and PGA values are clipped to the closest bound "if False":
PySeismoSoil/PySeismoSoil/class_site_factors.py
Lines 84 to 88 in b764057
| lenient : bool, default=False | |
| Whether to ensure the given Vs30, z1, and PGA values are within the | |
| valid range. If False and the given values fall outside the valid | |
| range, the given values (e.g., Vs30 = 170 m/s) will be treated as the | |
| closest boundary values (e.g., Vs30 = 175 m/s). |
PySeismoSoil/PySeismoSoil/class_site_effect_adjustment.py
Lines 35 to 39 in b764057
| lenient : bool, default=False | |
| Whether to ensure the given Vs30, z1, and PGA values are within the | |
| valid range. If False and the given values fall outside the valid | |
| range, the given values (e.g., Vs30 = 170 m/s) will be treated as the | |
| closest boundary values (e.g., Vs30 = 175 m/s). |
The code does the opposite. With lenient=False (the default) it raises ValueError; it clips only with lenient=True. Site_Effect_Adjustment passes lenient on to Site_Factors unchanged:
PySeismoSoil/PySeismoSoil/class_site_factors.py
Lines 144 to 170 in b764057
| if 'Vs30 out of range' in status: | |
| if not lenient: | |
| raise ValueError( | |
| 'Vs30 should be between ' | |
| f'[{MIN_VS30_M_S}, {MAX_VS30_M_S}] m/s' | |
| ) | |
| if Vs30_in_meter_per_sec < MIN_VS30_M_S: | |
| Vs30_in_meter_per_sec = MIN_VS30_M_S | |
| else: | |
| Vs30_in_meter_per_sec = MAX_VS30_M_S | |
| if 'z1 out of range' in status: | |
| if not lenient: | |
| raise ValueError( | |
| f'z1_in_m should be between [{MIN_Z1_M}, {MAX_Z1_M}] m' | |
| ) | |
| z1_in_m = MIN_Z1_M if z1_in_m < MIN_Z1_M else MAX_Z1_M | |
| if 'PGA out of range' in status: | |
| if not lenient: | |
| raise ValueError( | |
| f'PGA should be between [{MIN_PGA_G}g, {MAX_PGA_G}g]' | |
| ) | |
| PGA_in_g = MIN_PGA_G if PGA_in_g < MIN_PGA_G else MAX_PGA_G |
PySeismoSoil/PySeismoSoil/class_site_effect_adjustment.py
Lines 98 to 103 in b764057
| site_factor = Site_Factors( | |
| Vs30_in_meter_per_sec, | |
| z1_in_m, | |
| PGA_in_g, | |
| lenient=lenient, | |
| ) |
from PySeismoSoil.class_site_factors import Site_Factors
try:
Site_Factors(170, 100, 0.3, lenient=False)
except ValueError as e:
print('lenient=False ->', e)
# lenient=False -> Vs30 should be between [175, 950] m/s
sf = Site_Factors(170, 100, 0.3, lenient=True)
print('lenient=True ->', sf.Vs30)
# lenient=True -> 175Fix
- In both docstrings, describe the actual behavior, for example: "Whether to accept Vs30, z1, and PGA values outside the valid range. If
False, such values raise aValueError. IfTrue, they are treated as the closest boundary values (e.g., Vs30 = 170 m/s is treated as 175 m/s). This option does not apply to invalid Vs30-z1 combinations, which always raise aValueError." - No code or test change is needed.
Activity