fed.get_subscription_by_name doesn't work for subscriptions (unnamed inputs)

[trevorhardy]

Describe the bug

The federate class method "get_subscription_by_name" doesn't work for subscriptions since, by definition, they are unnamed inputs.

To Reproduce

Steps to reproduce the behavior:

Configure a federate using a JSON with the "subscription" object and try using the above method to get the subscription object. It will fail with a HELICS error saying there is no subscription by that name.

Environment (please complete the following information):

• Operating System: Mac 10.15.7
• Installation: pip install helics
• helics and pyhelics version: 3.3.0 for both

Share the output of the following:

$ python -c "import helics as h; import json; print(json.dumps(h.helicsGetSystemInfo(), indent=4, sort_keys=True))"


{
    "buildflags": " -O3 -DNDEBUG  $<$<COMPILE_LANGUAGE:CXX>:-std=c++17>",
    "compiler": "Unix Makefiles  Darwin-19.6.0:AppleClang-12.0.0.12000032",
    "cores": [
        "zmq",
        "zmqss",
        "tcp",
        "tcpss",
        "udp",
        "ipc",
        "interprocess",
        "inproc"
    ],
    "cpu": "Intel(R) Core(TM) i7-7920HQ CPU @ 3.10GHz",
    "cpucount": 8,
    "cputype": "x86_64",
    "hostname": "WE34262",
    "memory": "16384 MB",
    "os": "Darwin  19.6.0  Darwin Kernel Version 19.6.0: Tue Jun 21 21:18:39 PDT 2022; root:xnu-6153.141.66~1/RELEASE_X86_64",
    "version": {
        "build": "",
        "major": 3,
        "minor": 3,
        "patch": 0,
        "string": "3.3.0 (2022-09-15)"
    },
    "zmqversion": "ZMQ v4.3.4"
}

Additional context and information
Here's the logging output we're seeing with the error message:

Created federate transmission
      Number of subscriptions: 1
      Number of publications: 11
      Registered subscription---> distribution5/pcc.11.pq
Traceback (most recent call last):
  File "/home/ananth/code/NRECA/HELICS/multisite-demo/distribution5/transmission_dummy.py", line 62, in <module>
    subid[k] = fed.get_subscription_by_name(k)
  File "/home/ananth/.local/lib/python3.10/site-packages/helics/capi.py", line 2713, in get_subscription_by_name
    return helicsFederateGetInput(self, name)
  File "/home/ananth/.local/lib/python3.10/site-packages/helics/capi.py", line 7768, in helicsFederateGetInput
    raise HelicsException("[" + str(err.error_code) + "] " + ffi.string(err.message).decode())
helics.capi.HelicsException: [-4] the specified input name is a not a recognized input

capi.py for pyhelics adds the subscription objects to the federates "subscriptions" attribute (a dictionary keyed to the subscription target) by referencing the subscription objects by index; both inputs and subscriptions are put into the same dictionary and then the "inputs" attribute is equated to the "subscription" attribute. The above method takes a string (the subscription name) as an input and calls "helicsFederateGetInput()". This will always fail for subscriptions since they are, by definition, unnamed.

The best improvement we could make here is:

• Correctly populating inputs and subscriptions in the federate attributes so that they are unique dictionaries
• Adding a "n_inputs" attribute for the HelicsFederateClass
• Changing the name of "get_subscriptions_by_name" to "get_input_by_name"
• Probably other things I haven't thought of yet.

[kdheepak]

I think this would be a breaking change too.

[trevorhardy]

It looks like for the v3.3.1 update I'll be doing a partial update and adding warning messages that we'll be changing the functionality of .get_subscription_by_name() in v3.3.2.

[afisher1]

@trevorhardy so is the consensus that HelicsValueFederate should have get_input_by_name() and get_subscription_by_target() member functions?

[trevorhardy]

Yes, those two member functions would be good. For this next release, just add .get_input_by_name() and add a warning to .get_subscription_by_name() that indicates its functionality will be changed in the following release (two releases from now) such that it will only return subscription objects when using .get_subscription_by_name().

Here's how I see the total change rolling out for this release:

• HelicFederate class

  • Add a warning that the definition of .inputs will be changing in the next release. Right now it is defined to be identical to .subscriptions
  • Add a warning that the definition of .n_inputs will be changing in the next release. Right now it is defined to be identical to .n_subscriptions

• HelicsValueFederate

  • add .get_input_by_name()
  • add a warning to .get_subscription_by_name() that indicates its functionality will be changed in the following release

There's probably other things I'm missing so spend a little bit of time looking at those two classes and figuring out what needs to be a warning now so we can change it later and what we can add now without affecting the existing behavior. At its core, we're trying to disambiguate subscriptions and inputs in the API.