Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 41 additions & 51 deletions sbi/neural_nets/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@

from sbi.neural_nets.net_builders.estimator_configs import (
VF_MODELS,
_BUILD_KWARG_ALIASES,
_CLASSIFIER_CONFIGS,
_DENSITY_CONFIGS,
ConditionalFlowConfig,
MarginalFlowConfig,
_config_from_factory_kwargs,
_factory_defaults,
_mixed_config_from_factory_kwargs,
)
from sbi.neural_nets.net_builders.flow import (
build_made,
Expand Down Expand Up @@ -139,34 +138,11 @@ def _density_family_args(embedding_net: nn.Module, **family_args: Any) -> dict:
)


def _legacy_density_build_fn(
model: str,
family_args: dict,
extra: dict,
input_is_theta: bool,
) -> Callable:
"""Return a build function for the models that have no config yet.

``mnle`` and ``mnpe`` are reached through the density factories by the
deprecated string path of MNLE and MNPE, and are built from flat kwargs by
``build_mnle`` / ``build_mnpe`` rather than from a config.
"""
config = ConditionalFlowConfig.from_kwargs(
**{_BUILD_KWARG_ALIASES.get(k, k): v for k, v in family_args.items()},
**extra,
)
builder_kwargs = config.to_dict()
def _unknown_density_build_fn(model: str) -> Callable:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function removes the last production reader of model_builders, ConditionalFlowConfig and _BUILD_KWARG_ALIASES' flat path, so three things can be deleted I think, please double check:

  • model_builders at line 46, plus 16 of the 18 builder imports at line 19 that exist only to fill it (build_zuko_unconditional_flow is the only one with another caller). Not in sbi.neural_nets.__all__, not documented — about 34 lines kept alive by test_legacy_public_builder_registry_remains_complete.
  • ClassifierConfig (estimator_configs.py:358, 20 lines) — no reference left anywhere.
  • ConditionalFlowConfig (estimator_configs.py:298, 60 lines) — none in sbi/. The three tests in build_context_test.py that use it are testing _EstimatorBuilderBase, and MarginalFlowConfig is still alive, so it can stand in.

"""Preserve the factories' build-time error for unknown model names."""

def build_fn(batch_theta, batch_x):
if model not in model_builders:
raise NotImplementedError(f"Model {model} is not implemented")

modeled, condition = (
(batch_theta, batch_x) if input_is_theta else (batch_x, batch_theta)
)
return model_builders[model](
batch_x=modeled, batch_y=condition, **builder_kwargs
)
raise NotImplementedError(f"Model {model} is not implemented")

return build_fn

Expand Down Expand Up @@ -299,19 +275,27 @@ def likelihood_nn(
embedding_net=embedding_net,
num_components=num_components,
)
if model not in _DENSITY_CONFIGS:
return _legacy_density_build_fn(
model, family_args, kwargs, input_is_theta=False
if model in ("mnle", "mnpe"):
config = _mixed_config_from_factory_kwargs(
family_args=family_args,
factory_defaults=_factory_defaults(
likelihood_nn, _LIKELIHOOD_FACTORY_FIELDS
),
extra=kwargs,
)

config = _config_from_factory_kwargs(
model,
_DENSITY_CONFIGS,
"density",
family_args=family_args,
factory_defaults=_factory_defaults(likelihood_nn, _LIKELIHOOD_FACTORY_FIELDS),
extra=kwargs,
)
elif model in _DENSITY_CONFIGS:
config = _config_from_factory_kwargs(
model,
_DENSITY_CONFIGS,
"density",
family_args=family_args,
factory_defaults=_factory_defaults(
likelihood_nn, _LIKELIHOOD_FACTORY_FIELDS
),
extra=kwargs,
)
else:
return _unknown_density_build_fn(model)

def build_fn(batch_theta, batch_x):
# NLE models p(x|theta), so the modeled variable is x.
Expand Down Expand Up @@ -411,17 +395,23 @@ def build_fn_snpe_a(batch_theta, batch_x, num_components):

return build_fn_snpe_a

if model not in _DENSITY_CONFIGS:
return _legacy_density_build_fn(model, family_args, kwargs, input_is_theta=True)

config = _config_from_factory_kwargs(
model,
_DENSITY_CONFIGS,
"density",
family_args=family_args,
factory_defaults=_factory_defaults(posterior_nn, _POSTERIOR_FACTORY_FIELDS),
extra=kwargs,
)
if model in ("mnle", "mnpe"):
config = _mixed_config_from_factory_kwargs(
family_args=family_args,
factory_defaults=_factory_defaults(posterior_nn, _POSTERIOR_FACTORY_FIELDS),
extra=kwargs,
)
elif model in _DENSITY_CONFIGS:
config = _config_from_factory_kwargs(
model,
_DENSITY_CONFIGS,
"density",
family_args=family_args,
factory_defaults=_factory_defaults(posterior_nn, _POSTERIOR_FACTORY_FIELDS),
extra=kwargs,
)
else:
return _unknown_density_build_fn(model)

def build_fn(batch_theta, batch_x):
# NPE models p(theta|x), so the modeled variable is theta.
Expand Down
100 changes: 90 additions & 10 deletions sbi/neural_nets/net_builders/estimator_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1535,16 +1535,7 @@ def build(
return _build_mixed_density_estimator(
batch_x=batch_input,
batch_y=batch_condition,
continuous_config=self.continuous,
z_score_y=self.z_score_condition,
num_categories_per_variable=self.num_categories_per_variable,
embedding_net=self.embedding_net,
combined_embedding_net=self.combined_embedding_net,
log_transform_x=self.log_transform_x,
discrete_hidden_features=self.discrete_hidden_features,
discrete_hidden_layers=self.discrete_hidden_layers,
combined_embedding_features=self.combined_embedding_features,
dropout_probability=self.dropout_probability,
config=self,
)


Expand Down Expand Up @@ -1637,3 +1628,92 @@ def _config_from_factory_kwargs(
)

return config_cls(**accepted, extra_kwargs=unknown)


def _mixed_config_from_factory_kwargs(
family_args: dict,
factory_defaults: dict,
extra: dict,
) -> MixedConfig:
"""Build a mixed config from the deprecated factories' flat arguments."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a full docstring here explaining what this function does.

extra = dict(extra)
flow_model = extra.pop("flow_model", None)
if flow_model is None:
flow_model = "nsf"
config_cls = _DENSITY_CONFIGS.get(flow_model)

mixed_fields = (
"num_categories_per_variable",
"combined_embedding_net",
"log_transform_x",
"discrete_hidden_features",
"discrete_hidden_layers",
"combined_embedding_features",
"dropout_probability",
)
# The flat path dropped a recognised None before building, so it still
# means unset. A name no model knows keeps its None, and with it the
# warning that catches a typo.
recognised = {"continuous_hidden_features", *mixed_fields}
if config_cls is not None:
recognised |= {f.name for f in fields(config_cls)}
extra = {
name: value
for name, value in extra.items()
if value is not None or name not in recognised
}

mixed_kwargs = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

z_score_condition and embedding_net are forwarded whatever the caller did, while
the continuous arguments below drop a value still at the factory default. Both
defaults agree with MixedConfig's today, so nothing is wrong right now — but a
divergence would silently override the config default, which is exactly how the
tabpfn z_score_condition bug worked. Please run these two through the same
at-default skip.

"z_score_condition": family_args["z_score_condition"],
"embedding_net": family_args["embedding_net"],
}
for name in mixed_fields:
if name in extra:
mixed_kwargs[name] = extra.pop(name)

continuous_args = {
name: family_args[name]
for name in (
"z_score_input",
"hidden_features",
"num_transforms",
"num_bins",
"num_components",
)
}
continuous_defaults = {name: factory_defaults[name] for name in continuous_args}
continuous_args = {
name: continuous_defaults[name] if value is None else value
for name, value in continuous_args.items()
}

if "continuous_hidden_features" in extra:
# The flat API let this override only the continuous width while the
# categorical width kept falling back to `hidden_features`.
Comment on lines +1691 to +1692

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments.

mixed_kwargs.setdefault(
"discrete_hidden_features", continuous_args["hidden_features"]
)
continuous_args["hidden_features"] = extra.pop("continuous_hidden_features")

dropout_probability = mixed_kwargs.get("dropout_probability")
if (
dropout_probability is not None
and config_cls is not None
and "dropout_probability" in {f.name for f in fields(config_cls)}
):
extra["dropout_probability"] = dropout_probability

# The flat mixed API passed `tail_bound` to every builder, so the models
# that read it saw 10.0 rather than their own narrower default.
Comment on lines +1706 to +1707

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments.

if config_cls is not None and "tail_bound" in {f.name for f in fields(config_cls)}:
extra.setdefault("tail_bound", 10.0)

continuous = _config_from_factory_kwargs(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

likelihood_nn("mnle", num_components=5) now raises

Argument(s) ['num_components'] are not used by model='nsf' and would be silently ignored. Configure the model directly with NSFConfig, or use `extra_kwargs` to forward library-specific options.

The caller wrote model="mnle" and never mentioned nsf, and NSFConfig alone is not where they need to go — it is MixedConfig(continuous=NSFConfig(...)). This argument was silently ignored before this PR, so the message ships with it. Please make this call site name the model the caller chose and point at MixedConfig; the role argument is already threaded through for the unknown-model branch.

flow_model,
_DENSITY_CONFIGS,
"mixed continuous density",
family_args=continuous_args,
factory_defaults=continuous_defaults,
extra=extra,
)
return MixedConfig(continuous=continuous, **mixed_kwargs)
Loading
Loading