Merge pull request Pyomo#3554 from jsiirola/amplfunc-duplicates

Avoid duplicate `AMPLFUNC` entries in `ipopt_v2`

Author: blnicho

Diff for: pyomo/contrib/solver/solvers/ipopt.py

@@ -46,6 +46,7 @@
 from pyomo.core.expr.numvalue import value
 from pyomo.core.base.suffix import Suffix
 from pyomo.common.collections import ComponentMap
+from pyomo.solvers.amplfunc_merge import amplfunc_merge
 
 logger = logging.getLogger(__name__)
 
@@ -357,9 +358,9 @@ def solve(self, model, **kwds):
                 # Get a copy of the environment to pass to the subprocess
                 env = os.environ.copy()
                 if nl_info.external_function_libraries:
-                    if env.get('AMPLFUNC'):
-                        nl_info.external_function_libraries.append(env.get('AMPLFUNC'))
-                    env['AMPLFUNC'] = "\n".join(nl_info.external_function_libraries)
+                    env['AMPLFUNC'] = amplfunc_merge(
+                        env, *nl_info.external_function_libraries
+                    )
                 # Write the opt_file, if there should be one; return a bool to say
                 # whether or not we have one (so we can correctly build the command line)
                 opt_file = self._write_options_file(

Diff for: pyomo/core/tests/unit/test_external.py

@@ -585,6 +585,38 @@ def test_solve_gsl_function_const_arg(self):
         res = opt.solve(model, tee=True)
         self.assertAlmostEqual(value(model.x), 0.1, 5)
 
+    @unittest.skipIf(
+        not check_available_solvers('ipopt_v2'),
+        "The 'ipopt_v2' solver is not available",
+    )
+    def test_solve_gsl_function(self):
+        DLL = find_GSL()
+        if not DLL:
+            self.skipTest("Could not find the amplgsl.dll library")
+        model = ConcreteModel()
+        model.z_func = ExternalFunction(library=DLL, function="gsl_sf_gamma")
+        model.x = Var(initialize=3, bounds=(1e-5, None))
+        model.o = Objective(expr=model.z_func(model.x))
+        opt = SolverFactory('ipopt_v2')
+        res = opt.solve(model, tee=True)
+        self.assertAlmostEqual(value(model.o), 0.885603194411, 7)
+
+    @unittest.skipIf(
+        not check_available_solvers('ipopt_v2'),
+        "The 'ipopt_v2' solver is not available",
+    )
+    def test_solve_gsl_function_const_arg(self):
+        DLL = find_GSL()
+        if not DLL:
+            self.skipTest("Could not find the amplgsl.dll library")
+        model = ConcreteModel()
+        model.z_func = ExternalFunction(library=DLL, function="gsl_sf_beta")
+        model.x = Var(initialize=1, bounds=(0.1, None))
+        model.o = Objective(expr=-model.z_func(1, model.x))
+        opt = SolverFactory('ipopt_v2')
+        res = opt.solve(model, tee=True)
+        self.assertAlmostEqual(value(model.x), 0.1, 5)
+
     @unittest.skipIf(
         not check_available_solvers('ipopt'), "The 'ipopt' solver is not available"
     )

Diff for: pyomo/solvers/amplfunc_merge.py

@@ -9,24 +9,54 @@
 #  This software is distributed under the 3-clause BSD License.
 #  ___________________________________________________________________________
 
+from pyomo.common.deprecation import relocated_module_attribute
 
-def amplfunc_string_merge(amplfunc, pyomo_amplfunc):
-    """Merge two AMPLFUNC variable strings eliminating duplicate lines"""
-    # Assume that the strings amplfunc and pyomo_amplfunc don't contain duplicates
-    # Assume that the path separator is correct for the OS so we don't need to
-    # worry about comparing Unix and Windows paths.
-    amplfunc_lines = amplfunc.split("\n")
-    existing = set(amplfunc_lines)
-    for line in pyomo_amplfunc.split("\n"):
-        # Skip lines we already have
-        if line not in existing:
-            amplfunc_lines.append(line)
-    # Remove empty lines which could happen if one or both of the strings is
-    # empty or there are two new lines in a row for whatever reason.
-    amplfunc_lines = [s for s in amplfunc_lines if s != ""]
-    return "\n".join(amplfunc_lines)
-
-
-def amplfunc_merge(env):
-    """Merge AMPLFUNC and PYOMO_AMPLFUNC in an environment var dict"""
-    return amplfunc_string_merge(env.get("AMPLFUNC", ""), env.get("PYOMO_AMPLFUNC", ""))
+relocated_module_attribute(
+    'amplfunc_string_merge',
+    'pyomo.solvers.amplfunc_merge.unique_paths',
+    version='6.9.2.dev0',
+    f_globals=globals(),
+)
+
+
+def unique_paths(*paths):
+    """Merge paths, eliminating duplicates.
+
+    Parameters
+    ----------
+    *path : str
+
+        Each argument is a string containing one or more paths.
+        Multiple libraries are separated by newlines.
+
+    Returns
+    -------
+    str : merged list of newline-separated paths.
+
+    """
+    funcs = {}
+    for src in paths:
+        for line in src.splitlines():
+            if not line:
+                continue
+            funcs[line] = None
+    return "\n".join(funcs)
+
+
+def amplfunc_merge(env, *funcs):
+    """Merge AMPLFUNC and PYOMO_AMPLFUNC environment variables with provided values
+
+    Paths are returned with entries from AMPLFUNC first, PYOMO_AMPLFUNC
+    second, and the user-provided arguments last.  Duplicates and empty
+    paths are filtered out.
+
+    Parameters
+    ----------
+    env : Dict[str, str]
+        Environment dictionary mapping environment variable names to values.
+
+    *funcs : str
+        Additional paths to combine.
+
+    """
+    return unique_paths(env.get("AMPLFUNC", ""), env.get("PYOMO_AMPLFUNC", ""), *funcs)

Diff for: pyomo/solvers/tests/checks/test_amplfunc_merge.py

@@ -10,14 +10,14 @@
 #  ___________________________________________________________________________
 
 import pyomo.common.unittest as unittest
-from pyomo.solvers.amplfunc_merge import amplfunc_string_merge, amplfunc_merge
+from pyomo.solvers.amplfunc_merge import unique_paths, amplfunc_merge
 
 
 class TestAMPLFUNCStringMerge(unittest.TestCase):
     def test_merge_no_dup(self):
         s1 = "my/place/l1.so\nanother/place/l1.so"
         s2 = "my/place/l2.so"
-        sm = amplfunc_string_merge(s1, s2)
+        sm = unique_paths(s1, s2)
         sm_list = sm.split("\n")
         self.assertEqual(len(sm_list), 3)
         # The order of lines should be maintained with the second string
@@ -29,40 +29,40 @@ def test_merge_no_dup(self):
     def test_merge_empty1(self):
         s1 = ""
         s2 = "my/place/l2.so"
-        sm = amplfunc_string_merge(s1, s2)
+        sm = unique_paths(s1, s2)
         sm_list = sm.split("\n")
         self.assertEqual(len(sm_list), 1)
         self.assertEqual(sm_list[0], "my/place/l2.so")
 
     def test_merge_empty2(self):
         s1 = "my/place/l2.so"
         s2 = ""
-        sm = amplfunc_string_merge(s1, s2)
+        sm = unique_paths(s1, s2)
         sm_list = sm.split("\n")
         self.assertEqual(len(sm_list), 1)
         self.assertEqual(sm_list[0], "my/place/l2.so")
 
     def test_merge_empty_both(self):
         s1 = ""
         s2 = ""
-        sm = amplfunc_string_merge(s1, s2)
+        sm = unique_paths(s1, s2)
         sm_list = sm.split("\n")
         self.assertEqual(len(sm_list), 1)
         self.assertEqual(sm_list[0], "")
 
     def test_merge_bad_type(self):
-        self.assertRaises(AttributeError, amplfunc_string_merge, "", 3)
-        self.assertRaises(AttributeError, amplfunc_string_merge, 3, "")
-        self.assertRaises(AttributeError, amplfunc_string_merge, 3, 3)
-        self.assertRaises(AttributeError, amplfunc_string_merge, None, "")
-        self.assertRaises(AttributeError, amplfunc_string_merge, "", None)
-        self.assertRaises(AttributeError, amplfunc_string_merge, 2.3, "")
-        self.assertRaises(AttributeError, amplfunc_string_merge, "", 2.3)
+        self.assertRaises(AttributeError, unique_paths, "", 3)
+        self.assertRaises(AttributeError, unique_paths, 3, "")
+        self.assertRaises(AttributeError, unique_paths, 3, 3)
+        self.assertRaises(AttributeError, unique_paths, None, "")
+        self.assertRaises(AttributeError, unique_paths, "", None)
+        self.assertRaises(AttributeError, unique_paths, 2.3, "")
+        self.assertRaises(AttributeError, unique_paths, "", 2.3)
 
     def test_merge_duplicate1(self):
         s1 = "my/place/l1.so\nanother/place/l1.so"
         s2 = "my/place/l1.so\nanother/place/l1.so"
-        sm = amplfunc_string_merge(s1, s2)
+        sm = unique_paths(s1, s2)
         sm_list = sm.split("\n")
         self.assertEqual(len(sm_list), 2)
         # The order of lines should be maintained with the second string
@@ -73,7 +73,7 @@ def test_merge_duplicate1(self):
     def test_merge_duplicate2(self):
         s1 = "my/place/l1.so\nanother/place/l1.so"
         s2 = "my/place/l1.so"
-        sm = amplfunc_string_merge(s1, s2)
+        sm = unique_paths(s1, s2)
         sm_list = sm.split("\n")
         self.assertEqual(len(sm_list), 2)
         # The order of lines should be maintained with the second string
@@ -84,7 +84,7 @@ def test_merge_duplicate2(self):
     def test_merge_extra_linebreaks(self):
         s1 = "\nmy/place/l1.so\nanother/place/l1.so\n"
         s2 = "\nmy/place/l1.so\n\n"
-        sm = amplfunc_string_merge(s1, s2)
+        sm = unique_paths(s1, s2)
         sm_list = sm.split("\n")
         self.assertEqual(len(sm_list), 2)
         # The order of lines should be maintained with the second string