Skip to content

fit_all_damping_curves(save_txt=True) checks func_serialize and txt_filename only after all the fitting is done #97

Open

Description

@jsh9

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

Problem

fit_all_damping_curves() fits every layer first, and only then looks at the save_txt options. A missing func_serialize raises ValueError after the fitting. A missing txt_filename (the default is None) is not checked at all: np.savetxt(None, ...) fails after the fitting. With the default pop_size=800 and n_gen=100, the fitted parameters are lost after a long run.

if parallel:
p = mp.Pool(n_cores)
params = p.map(
_fit_single_layer_loop,
itertools.product(curves_list, other_params),
)
else:
params = []
for curve in curves_list:
params.append(_fit_single_layer_loop((curve, other_params[0])))

if save_txt:
if func_serialize is None:
raise ValueError(
'Please provide a function to serialize the parameters into a'
' list.',
)
data_for_file = [func_serialize(param) for param in params]
data_for_file__ = np.column_stack(tuple(data_for_file))
np.savetxt(txt_filename, data_for_file__, fmt='%.6g', delimiter=sep)

import numpy as np
import PySeismoSoil.helper_hh_model as hh
import PySeismoSoil.helper_site_response as sr

curve = np.genfromtxt('tests/files/curve_FKSH14.txt')[:, 2:4]
n_fits = 0

def counting_fit(*args, **kwargs):
    global n_fits
    n_fits += 1
    return hh.fit_HH_x_single_layer(*args, **kwargs)

for kwargs in (
    {'func_serialize': None},
    {'func_serialize': hh.serialize_params_to_array},  # txt_filename=None
):
    n_fits = 0
    try:
        sr.fit_all_damping_curves(
            [curve] * 3, counting_fit, hh.tau_HH,
            pop_size=5, n_gen=5, save_txt=True, **kwargs,
        )
    except ValueError as e:
        print(n_fits, e)
# 3 Please provide a function to serialize the parameters into a list.  (expected 0 fits)
# 3 fname must be a string or file handle  (expected 0 fits, and a clear message)

The existing test test_fit_all_damping_curves__exception_when_no_func_serialize on main (in #74: test_fit_all_damping_curves__exception[no_func_serialize]) only checks that the ValueError is raised, not when.

Also:

  • save_fig=True saves nothing unless show_fig=True too, because the figure is only built, and only saved, when show_fig is True (if show_fig and save_fig:). The docstring of save_fig does not say this. Multiple_Damping_Curves.get_all_HH_x_params() and get_all_H4_x_params() pass save_fig through, so they also save nothing when show_fig=False.
  • show_fig=True, save_fig=True with the default fig_filename=None fails after the fitting, inside fig.savefig(None).

save_fig : bool, default=False
Whether to save damping fitting figures to hard drive.
fig_filename : str | None, default=None
Full file name of the figure.

if show_fig:
ncol = 4
nrow = int(np.ceil(len(curves_list) / ncol))
fig = plt.figure(figsize=(ncol * 3, nrow * 3))
for j, curve in enumerate(curves_list):
ax = plt.subplot(nrow, ncol, j + 1)
_plot_damping_curve_fit(
curve,
params[j],
func_stress,
fig=fig,
ax=ax,
)
fig.tight_layout(pad=0.5, h_pad=0.5, w_pad=0.5)
if show_fig and save_fig:
fig.savefig(fig_filename, dpi=dpi)

params = sr.fit_all_damping_curves(
list_of_np_array,
hh.fit_HH_x_single_layer,
hh.tau_HH,
use_scipy=use_scipy,
pop_size=pop_size,
n_gen=n_gen,
lower_bound_power=lower_bound_power,
upper_bound_power=upper_bound_power,
eta=eta,
seed=seed,
show_fig=show_fig,
verbose=verbose,
parallel=parallel,
n_cores=n_cores,
save_fig=save_fig,
fig_filename=fig_filename,
dpi=dpi,
save_txt=save_txt,
txt_filename=txt_filename,
sep=sep,
func_serialize=hh.serialize_params_to_array,
)

params = sr.fit_all_damping_curves(
list_of_np_array,
mkz.fit_H4_x_single_layer,
mkz.tau_MKZ,
use_scipy=use_scipy,
pop_size=pop_size,
n_gen=n_gen,
lower_bound_power=lower_bound_power,
upper_bound_power=upper_bound_power,
eta=eta,
seed=seed,
show_fig=show_fig,
verbose=verbose,
parallel=parallel,
n_cores=n_cores,
save_fig=save_fig,
fig_filename=fig_filename,
dpi=dpi,
save_txt=save_txt,
txt_filename=txt_filename,
sep=sep,
func_serialize=mkz.serialize_params_to_array,
)

import os
import tempfile
import numpy as np
import PySeismoSoil.helper_hh_model as hh
import PySeismoSoil.helper_site_response as sr
from PySeismoSoil.class_curves import Multiple_Damping_Curves

curve = np.genfromtxt('tests/files/curve_FKSH14.txt')[:, 2:4]
fig_file = os.path.join(tempfile.mkdtemp(), 'fit.png')
sr.fit_all_damping_curves(
    [curve], hh.fit_HH_x_single_layer, hh.tau_HH,
    pop_size=1, n_gen=1, save_fig=True, fig_filename=fig_file,
)
print(os.path.exists(fig_file))  # False  (expected True)

mdc = Multiple_Damping_Curves('tests/files/curve_FKSH14.txt')
mdc.get_all_HH_x_params(pop_size=1, n_gen=1, save_fig=True, fig_filename=fig_file)
print(os.path.exists(fig_file))  # False  (expected True)
mdc.get_all_H4_x_params(pop_size=1, n_gen=1, save_fig=True, fig_filename=fig_file)
print(os.path.exists(fig_file))  # False  (expected True)

sr.fit_all_damping_curves(
    [curve], hh.fit_HH_x_single_layer, hh.tau_HH,
    pop_size=1, n_gen=1, show_fig=True, save_fig=True,
)
# AttributeError: 'NoneType' object has no attribute 'write'

Fix

  1. At the start of fit_all_damping_curves(), before any fitting, raise ValueError when:
    • save_txt is True and func_serialize is None (move the existing check here);
    • save_txt is True and txt_filename is None;
    • save_fig is True and fig_filename is None.
  2. Build the figure when show_fig or save_fig, and save it when save_fig (instead of show_fig and save_fig).
  3. Tests: add {'func_serialize': hh.serialize_params_to_array} (no txt_filename) as a ValueError case to test_fit_all_damping_curves__exception (Combine near-duplicate test functions into parametrized tests #74), and pass a fitting function that fails the test if it is called, so the cases check that no fitting runs. Add a test that save_fig=True, fig_filename=tmp_path / 'fit.png' writes the file with show_fig=False.

With only steps 1 and 2 on main, both repros above give the expected results (0 fits before each error, and the figure files are written), 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