From be41d0f814f255687daf0be7600f6746b0e8e6e0 Mon Sep 17 00:00:00 2001
From: Christoph Groth <christoph.groth@cea.fr>
Date: Thu, 28 Mar 2019 14:26:15 +0100
Subject: [PATCH] relax unnecessary restrictions on signatures of value
functions
---
doc/source/pre/whatsnew/1.4.rst | 47 ++++++++++-------------
kwant/builder.py | 67 +++++++++++++++++++++++++--------
2 files changed, 72 insertions(+), 42 deletions(-)
diff --git a/doc/source/pre/whatsnew/1.4.rst b/doc/source/pre/whatsnew/1.4.rst
index a06e5e55..22ed7dc7 100644
--- a/doc/source/pre/whatsnew/1.4.rst
+++ b/doc/source/pre/whatsnew/1.4.rst
@@ -22,8 +22,7 @@ Summary: release highlights
Backwards-incompatible changes:
-* `Value functions may no longer take unnamed arguments`_.
-* `Value functions may no longer have default values for parameters`_.
+* `Restrictions on value functions when named parameters are given`_
.. _whatsnew14-magnetic:
@@ -261,31 +260,22 @@ lattices to the discretizer::
This is useful when you need a finer discretization step in some spatial
directions, and a coarser one in others.
-Value functions may no longer take unnamed arguments
-----------------------------------------------------
-The system parameter (``args`` and ``params``) handling of Kwant has been
-rewritten following the principle that each value function must take a clearly
-specified set of named parameters. This allows to make the parameter handling
-less error-prone (as explained in the following section) and faster. For this
-reason, value functions may no longer accept ``*args`` or ``**kwargs``.
+Restrictions on value functions when named parameters are given
+---------------------------------------------------------------
+New restrictions apply to how value functions may accept arguments, when named
+parameters are given through ``params``. (Nothing changes when the now
+deprcated ``args`` mechanism is used). The restrictions follow the principle
+that each value function must take a clearly specified set of named parameters.
+This allows to make the parameter handling less error-prone and faster.
-If you are using such functions, we recommend that you replace them by named
-parameters.
+In particular, when ``params`` is used, it is no longer possible for value
+functions to
+- take ``*args`` or ``**kwargs``,
+- take keyword-only parameters,
+- have default parameters for arguments.
-Value functions may no longer have default values for parameters
-----------------------------------------------------------------
-Using value functions with default values for parameters can be
-problematic, especially when re-using value functions between simulations.
-When parameters have default values it is easy to forget that such a
-parameter exists at all, because it is not necessary to provide them explicitly
-to functions that use the Kwant system. This means that other value functions
-might be introduced that also depend on the same parameter,
-but in an inconsistent way (e.g. a parameter 'phi' that is a superconducting
-phase in one value function, but a peierls phase in another). This leads
-to bugs that are confusing and hard to track down.
-
-For this reason value functions may no longer have default values for paramters.
-Concretely this means that the following no longer works::
+As an example, the following snippet no longer works because it uses default
+values::
syst = kwant.Builder()
@@ -339,8 +329,11 @@ of Ubuntu and Debian GNU/Linux.
Changes in Kwant 1.4.1
----------------------
- The list of user-visible changes was rearranged to emphasize
- backwards-incompatible changes by moving them to the top of the list and
- adding the entry `Value functions may no longer take unnamed arguments`_.
+ backwards-incompatible changes by moving them to the top of the list.
+- Restrictions on value functions no longer apply when the old ``args``
+ mechanism is used, this restores most of the backwards compatibility with
+ previous Kwant versions: `Restrictions on value functions when named
+ parameters are given`_.
- Kwant no longer requires the existence of a `parameters` attribute for
low-level systems.
- A note about an :ref:`whatsnew13-params-api-change` that ocurred in Kwant
diff --git a/kwant/builder.py b/kwant/builder.py
index 8eb11c43..ad482a75 100644
--- a/kwant/builder.py
+++ b/kwant/builder.py
@@ -1868,6 +1868,9 @@ class _FinalizedBuilderMixin:
if param_names is not None: # 'value' is callable
site = self.symmetry.to_fd(self.sites[i])
if params:
+ # See body of _value_params_pair_cache().
+ if isinstance(param_names, Exception):
+ raise param_names
args = map(params.__getitem__, param_names)
try:
value = value(site, *args)
@@ -1891,6 +1894,9 @@ class _FinalizedBuilderMixin:
sites = self.sites
site_i, site_j = self.symmetry.to_fd(sites[i], sites[j])
if params:
+ # See body of _value_params_pair_cache().
+ if isinstance(param_names, Exception):
+ raise param_names
args = map(params.__getitem__, param_names)
try:
value = value(site_i, site_j, *args)
@@ -1940,7 +1946,18 @@ def _value_params_pair_cache(nstrip):
if isinstance(value, _Substituted):
entry = value.func, value.params[nstrip:]
elif callable(value):
- entry = value, get_parameters(value)[nstrip:]
+ try:
+ param_names = get_parameters(value)
+ except ValueError as ex:
+ # The parameter names are determined and stored in advance
+ # for future use. This has failed, but it will only turn
+ # into a problem if user code ever uses the 'params'
+ # mechanism. To maintain backwards compatibility, we catch
+ # and store the exception so that it can be raised whenever
+ # appropriate.
+ entry = value, ex
+ else:
+ entry = value, param_names[nstrip:]
else:
# None means: value is not callable. (That's faster to check.)
entry = value, None
@@ -2036,13 +2053,23 @@ class FiniteSystem(_FinalizedBuilderMixin, system.FiniteSystem):
hoppings = [cache(builder._get_edge(sites[tail], sites[head]))
for tail, head in g]
- # System parameters are the union of the parameters
- # of onsites and hoppings.
- # Here 'onsites' and 'hoppings' are pairs whos second element
- # is a tuple of parameter names when matrix element is a function,
- # and None otherwise.
- parameters = frozenset(chain.from_iterable(
- p for _, p in chain(onsites, hoppings) if p))
+ # Compute the union of the parameters of onsites and hoppings. Here,
+ # 'onsites' and 'hoppings' are pairs whose second element is one of
+ # three things:
+ #
+ # * a tuple of parameter names when the matrix element is a function,
+ # * 'None' when it is a constant,
+ # * an exception when the parameter names could not have been
+ # determined (See body of _value_params_pair_cache()).
+ parameters = []
+ for _, names in chain(onsites, hoppings):
+ if isinstance(names, Exception):
+ parameters = None
+ break
+ if names:
+ parameters.extend(names)
+ else:
+ parameters = frozenset(parameters)
self.graph = g
self.sites = sites
@@ -2205,13 +2232,23 @@ class InfiniteSystem(_FinalizedBuilderMixin, system.InfiniteSystem):
tail, head = sym.to_fd(tail, head)
hoppings.append(cache(builder._get_edge(tail, head)))
- # System parameters are the union of the parameters
- # of onsites and hoppings.
- # Here 'onsites' and 'hoppings' are pairs whos second element
- # is a tuple of parameter names when matrix element is a function,
- # and None otherwise.
- parameters = frozenset(chain.from_iterable(
- p for _, p in chain(onsites, hoppings) if p))
+ # Compute the union of the parameters of onsites and hoppings. Here,
+ # 'onsites' and 'hoppings' are pairs whose second element is one of
+ # three things:
+ #
+ # * a tuple of parameter names when the matrix element is a function,
+ # * 'None' when it is a constant,
+ # * an exception when the parameter names could not have been
+ # determined (See body of _value_params_pair_cache()).
+ parameters = []
+ for _, names in chain(onsites, hoppings):
+ if isinstance(names, Exception):
+ parameters = None
+ break
+ if names:
+ parameters.extend(names)
+ else:
+ parameters = frozenset(parameters)
self.graph = g
self.sites = sites
--
GitLab