Merge branch 'fix_conversion_io' into develop

Author: LourensVeen

ymmsl/command_line.py

@@ -1,7 +1,8 @@
 import click
 import os
 from shutil import copyfile
-from typing import Optional
+from typing import Optional, TextIO, Type, Union
+import warnings
 
 from ymmsl.conversion.converter import DowngradeError
 from ymmsl.document import Document
@@ -10,10 +11,13 @@
 import ymmsl.v0_2 as v0_2
 
 
-_version_tag_to_type = {
-        'v0.1': v0_1.PartialConfiguration,
-        'v0.2': v0_2.Configuration,
-        }
+def showwarning(
+        message: Union[Warning, str], category: Type[Warning], filename: str,
+        lineno: int, file: Optional[TextIO] = None, line: Optional[str] = None) -> None:
+    print(f'WARNING: {message}', file=file)
+
+
+warnings.showwarning = showwarning
 
 
 @click.group()
@@ -27,6 +31,12 @@ def ymmsl() -> None:
     pass
 
 
+_version_tag_to_type = {
+        'v0.1': v0_1.PartialConfiguration,
+        'v0.2': v0_2.Configuration,
+        }
+
+
 @ymmsl.command(short_help='Convert a yMMSL file to a newer version')
 @click.argument(
         'input_file', default='-', type=click.Path(
@@ -87,6 +97,9 @@ def convert(
     won't work, because the shell will open the file to do the redirect, empty it, and
     then run ymmsl convert, which then fails because the input is empty.
     """
+    if input_file != '-':
+        print(f'Converting {input_file}')
+
     if output_file is None:
         output_file = input_file
 
@@ -113,7 +126,11 @@ def convert(
     if output_file != '-' and os.path.exists(output_file):
         backup_file = output_file + '.bak'
         if not os.path.exists(backup_file):
-            copyfile(output_file, output_file + '.bak')
+            copyfile(output_file, backup_file)
+            print(f'Wrote backup file {backup_file}')
 
     with click.open_file(output_file, 'w') as output_stream:
         save(document, output_stream)
+
+    if input_file != '-':
+        print('Conversion complete')

ymmsl/conversion/convert_v0_1_to_v0_2.py

@@ -1,5 +1,5 @@
 from copy import deepcopy
-from typing import Dict, Optional
+from typing import Dict, List, Optional
 import warnings
 
 import ymmsl.v0_1 as v0_1
@@ -15,11 +15,21 @@ def convert_v0_1_to_v0_2(config: v0_1.PartialConfiguration) -> v0_2.Configuratio
     Returns:
         The corresponding configuration expressed in yMMSL v0.2.
     """
-    description = '' if config.description is None else config.description
+    description = 'Please add a description'
+    if config.description:
+        description = config.description
     models = [convert_model(config.model)] if config.model is not None else None
     settings = deepcopy(config.settings)
     programs = [
             convert_implementation(impl) for impl in config.implementations.values()]
+    if programs:
+        warnings.warn(
+                'In yMMSL v0.2 implementations have become programs, and you can now'
+                ' specify the ports of a program in the yMMSL description. If your'
+                ' program has fixed ports then you should do this, because it will make'
+                ' incorrect wiring easier to debug. While there, add a description too!'
+                )
+
     resources = deepcopy(config.resources)
     checkpoints = deepcopy(config.checkpoints)
     resume = deepcopy(config.resume)
@@ -32,20 +42,57 @@ def convert_v0_1_to_v0_2(config: v0_1.PartialConfiguration) -> v0_2.Configuratio
 def convert_component(component: v0_1.Component) -> v0_2.Component:
     """Convert a v0.1 Component object to a v0.2 Component."""
     ports = component.ports if component.ports else v0_1.Ports()
+    description = 'Please add a description'
     implementation: Optional[str] = None
     if component.implementation is not None:
         implementation = str(component.implementation)
 
     return v0_2.Component(
-            str(component.name), convert_ports(ports), '', False, implementation,
-            component.multiplicity)
+            str(component.name), convert_ports(ports), description, False,
+            implementation, component.multiplicity)
 
 
 def convert_conduit(conduit: v0_1.Conduit) -> v0_2.Conduit:
     """Convert a v0.1 Conduit to a v0.2 Conduit."""
     return v0_2.Conduit(str(conduit.sender), str(conduit.receiver))
 
 
+def infer_ports(components: List[v0_2.Component], conduits: List[v0_2.Conduit]) -> None:
+    """Infer component ports from conduits where absent.
+
+    This can create an incorrect result, because we have to guess at the operators. We
+    assume that they're F_INIT for receiving ports and O_F for sending ports, but this
+    can be wrong. Only the user knows, so we'll warn them that they have to check.
+    """
+    changed_components = list()
+    for component in components:
+        if len(component.ports) == 0:
+            for conduit in conduits:
+                if conduit.sending_component() == component.name:
+                    component.ports[conduit.sending_port()] = v0_2.Port(
+                            conduit.sending_port(), v0_2.Operator.O_F,
+                            v0_2.Timeline(''))
+                    if component.name not in changed_components:
+                        changed_components.append(component.name)
+
+                if conduit.receiving_component() == component.name:
+                    component.ports[conduit.receiving_port()] = v0_2.Port(
+                            conduit.receiving_port(), v0_2.Operator.F_INIT,
+                            v0_2.Timeline(''))
+                    if component.name not in changed_components:
+                        changed_components.append(component.name)
+
+    if changed_components:
+        ch_comp_list = '\n - ' + '\n - '.join(map(str, changed_components))
+        warnings.warn(
+                'In yMMSL v0.2 components are required to declare their ports. The'
+                ' following components did not have a ports declaration, so one has'
+                ' been added based on the connected conduits. THIS MAY BE WRONG,'
+                ' because the operators have all been set to F_INIT and O_F, while they'
+                ' may really be O_I or S. Please check these components and adjust'
+                f' as needed: {ch_comp_list}')
+
+
 def convert_model(model: v0_1.ModelReference) -> v0_2.Model:
     """Convert a v0.1 ModelReference object to a v0.2 Model.
 
@@ -55,13 +102,16 @@ def convert_model(model: v0_1.ModelReference) -> v0_2.Model:
     Returns:
         The corresponding configuration expressed in yMMSL v0.2.
     """
+    description = 'Please add a description'
+
     if isinstance(model, v0_1.Model):
+        components = list(map(convert_component, model.components))
+        conduits = list(map(convert_conduit, model.conduits))
+        infer_ports(components, conduits)
         return v0_2.Model(
-                str(model.name), None, '', None,
-                list(map(convert_component, model.components)),
-                list(map(convert_conduit, model.conduits)))
+                str(model.name), None, description, None, components, conduits)
     else:
-        return v0_2.Model(str(model.name), None, '', None, [], [])
+        return v0_2.Model(str(model.name), None, description, None, [], [])
 
 
 def convert_implementation(impl: v0_1.Implementation) -> v0_2.Program:
@@ -76,12 +126,7 @@ def convert_implementation(impl: v0_1.Implementation) -> v0_2.Program:
     Returns:
         The corresponding program expressed in yMMSL v0.2.
     """
-    warnings.warn(
-            'In yMMSL v0.2 implementations have become programs, and you can now'
-            ' specify the ports of a program in the yMMSL description. If your program'
-            ' has fixed ports then you should do this, because it will make incorrect'
-            ' wiring easier to debug. While there, add a description too!')
-
+    description = 'Please add a description'
     base_env: Optional[v0_1.BaseEnv] = impl.base_env
     env: Optional[Dict[str, str]] = impl.env
 
@@ -94,7 +139,7 @@ def convert_implementation(impl: v0_1.Implementation) -> v0_2.Program:
             env = None
 
     return v0_2.Program(
-                str(impl.name), None, '', None, base_env, impl.modules,
+                str(impl.name), None, description, None, base_env, impl.modules,
                 impl.virtual_env, env, execution_model, impl.executable, impl.args,
                 impl.script, impl.can_share_resources, impl.keeps_state_for_next_use)
 

ymmsl/conversion/tests/test_convert_v0_1_to_v0_2.py

@@ -16,7 +16,7 @@
 
 def test_convert_simple_config(empty_config: v0_1.PartialConfiguration) -> None:
     v2 = convert_v0_1_to_v0_2(empty_config)
-    assert v2.description == ''
+    assert v2.description == 'Please add a description'
     assert isinstance(v2.settings, v0_2.Settings)
     assert len(v2.settings) == 0
     assert v2.resources == {}

ymmsl/io.py

@@ -27,10 +27,11 @@
         v0_2.BaseEnv, v0_2.CheckpointRangeRule, v0_2.CheckpointAtRule,
         v0_2.CheckpointRule, v0_2.Checkpoints, v0_2.Component, v0_2.Conduit,
         v0_2.Configuration, v0_2.Document, v0_2.ExecutionModel, v0_2.Identifier,
-        v0_2.ImportKind, v0_2.ImportStatement, v0_2.KeepsStateForNextUse, v0_2.Model,
-        v0_2.MPICoresResReq, v0_2.MPINodesResReq, v0_2_MulticastConduit, v0_2.Ports,
-        v0_2.Program, v0_2.Reference, v0_2.ResourceRequirements, v0_2.SettingType,
-        v0_2.Settings, v0_2.SupportedSettings, v0_2.ThreadedResReq)
+        v0_2.Implementation, v0_2.ImportKind, v0_2.ImportStatement,
+        v0_2.KeepsStateForNextUse, v0_2.Model, v0_2.MPICoresResReq, v0_2.MPINodesResReq,
+        v0_2_MulticastConduit, v0_2.Ports, v0_2.Program, v0_2.Reference,
+        v0_2.ResourceRequirements, v0_2.SettingType, v0_2.Settings,
+        v0_2.SupportedSettings, v0_2.ThreadedResReq)
 
 
 _load = yatiml.load_function(*_classes)

ymmsl/v0_2/configuration.py

@@ -95,7 +95,7 @@ def check_duplicate_impl_names(
             self.models: MutableMapping[Reference, Model] = dict()
         elif isinstance(models, abc.Sequence):
             check_duplicate_impl_names('models', models)
-            self.models = {model.name: model for model in models}
+            self.models = {copy(model.name): model for model in models}
         elif isinstance(models, Model):
             self.models = {models.name: models}
         else:
@@ -531,6 +531,7 @@ def _yatiml_sweeten(cls, node: yatiml.Node) -> None:
         models = node.get_attribute('models')
         if (models.is_scalar(type(None)) or models.is_mapping() and models.is_empty()):
             node.remove_attribute('models')
+        node.index_attribute_to_map('models', 'name')
 
         if node.get_attribute('custom_implementations').is_scalar(type(None)):
             node.remove_attribute('custom_implementations')

ymmsl/v0_2/implementation.py

@@ -58,3 +58,8 @@ def _yatiml_sweeten(cls, node: yatiml.Node) -> None:
             ynode.style = '|'
             if not ynode.value.endswith('\n'):
                 ynode.value += '\n'
+
+        if len(node.get_attribute('supported_settings').yaml_node.value) == 0:
+            node.remove_attribute('supported_settings')
+
+        node.remove_attributes_with_default_values(cls)