Harmonize advanced default and fundamental integration examples

[eranschweitzer]

Summary

If merged this pull request will close Issue #54 and the fundamental integration and the advanced default examples will produce the same results, as expected/desired.

Proposed changes

• Make sure that the random seed calls in all federates are the same
• Change the requested time for the controller in fundamental integration to HELICS_TIME_MAXTIME rather than int(HELICS_TIME_MAXTIME) which seems to have led to the charger and battery processes hanging.
• Changes the runner files for both examples to remove the auto broker, which is currently not supported.

[eranschweitzer]

@kdheepak there appear to be some issues with the various pyhelics updates being made.

Here is my run from yesterday for the fundamental integration example:

(helics32) C:\Users\schw197\OneDrive - PNNL\Documents\HELICS-Examples>helics --version
helics, version v3.2.1.post3

Python HELICS version v3.2.1.post3       

HELICS Library version 3.2.1 (2022-06-16)

(helics32) C:\Users\schw197\OneDrive - PNNL\Documents\HELICS-Examples>cd user_guide_examples\fundamental\fundamental_integration  

(helics32) C:\Users\schw197\OneDrive - PNNL\Documents\HELICS-Examples\user_guide_examples\fundamental\fundamental_integration>helics run --path=fundamental_integration_runner.json  
Running federation: fundamental_integration
Running federate broker as a background process
Running federate Charger as a background process
Running federate Controller as a background process
Running federate Battery as a background process
Waiting for 4 processes to finish ...
Done.

Everything works just fine.

Now, I updated to helics:

(helics) C:\Users\schw197\OneDrive - PNNL\Documents\HELICS-Examples>pip install --upgrade "helics[cli]"
Requirement already satisfied: helics[cli] in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (3.2.1.post3)
Collecting helics[cli]
  Downloading helics-3.2.1.post8-py3-none-win_amd64.whl (11.3 MB)
     |████████████████████████████████| 11.3 MB 6.4 MB/s
Requirement already satisfied: strip-hints in c:\users\schw197\appdata\roaming\python\python39\site-packages (from helics[cli]) (0.1.10)
Requirement already satisfied: click>=8 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from helics[cli]) (8.0.3)
Requirement already satisfied: cffi>=1.6.0 in c:\users\schw197\appdata\roaming\python\python39\site-packages (from helics[cli]) (1.15.1)
Requirement already satisfied: matplotlib in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from helics[cli]) (3.5.1)
Requirement already satisfied: requests in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from helics[cli]) (2.27.1)
Requirement already satisfied: flask-cors in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from helics[cli]) (3.0.10)
Requirement already satisfied: pandas in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from helics[cli]) (1.4.3)
Requirement already satisfied: flask-restful in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from helics[cli]) (0.3.9)
Requirement already satisfied: flask>=2 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from helics[cli]) (2.0.2)
Requirement already satisfied: SQLAlchemy in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from helics[cli]) (1.4.32)
Requirement already satisfied: pycparser in c:\users\schw197\appdata\roaming\python\python39\site-packages (from cffi>=1.6.0->helics[cli]) (2.21)
Requirement already satisfied: colorama in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from click>=8->helics[cli]) (0.4.4)
Requirement already satisfied: itsdangerous>=2.0 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from flask>=2->helics[cli]) (2.0.1)
Requirement already satisfied: Werkzeug>=2.0 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from flask>=2->helics[cli]) (2.0.2)
Requirement already satisfied: Jinja2>=3.0 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from flask>=2->helics[cli]) (3.0.2)
Requirement already satisfied: MarkupSafe>=2.0 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from Jinja2>=3.0->flask>=2->helics[cli]) (2.0.1)
Requirement already satisfied: Six in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from flask-cors->helics[cli]) (1.16.0)
Requirement already satisfied: pytz in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from flask-restful->helics[cli]) (2021.3)
Requirement already satisfied: aniso8601>=0.82 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from flask-restful->helics[cli]) (9.0.1)
Requirement already satisfied: kiwisolver>=1.0.1 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from matplotlib->helics[cli]) (1.3.2)
Requirement already satisfied: pyparsing>=2.2.1 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from matplotlib->helics[cli]) (3.0.4)
Requirement already satisfied: fonttools>=4.22.0 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from matplotlib->helics[cli]) (4.28.5)
Requirement already satisfied: numpy>=1.17 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from matplotlib->helics[cli]) (1.22.1)
Requirement already satisfied: pillow>=6.2.0 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from matplotlib->helics[cli]) (9.0.0)
Requirement already satisfied: cycler>=0.10 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from matplotlib->helics[cli]) (0.11.0)
Requirement already satisfied: python-dateutil>=2.7 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from matplotlib->helics[cli]) (2.8.2)
Requirement already satisfied: packaging>=20.0 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from matplotlib->helics[cli]) (21.3)
Requirement already satisfied: idna<4,>=2.5 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from requests->helics[cli]) (3.3)
Requirement already satisfied: certifi>=2017.4.17 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from requests->helics[cli]) (2021.10.8)
Requirement already satisfied: charset-normalizer~=2.0.0 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from requests->helics[cli]) (2.0.4)
Requirement already satisfied: urllib3<1.27,>=1.21.1 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from requests->helics[cli]) (1.26.7)
Requirement already satisfied: greenlet!=0.4.17 in c:\users\schw197\miniconda3\envs\helics\lib\site-packages (from SQLAlchemy->helics[cli]) (1.1.1)
Requirement already satisfied: wheel in c:\users\schw197\appdata\roaming\python\python39\site-packages (from strip-hints->helics[cli]) (0.37.1)
Installing collected packages: helics
  Attempting uninstall: helics
    Found existing installation: helics 3.2.1.post3
    Uninstalling helics-3.2.1.post3:
      Successfully uninstalled helics-3.2.1.post3
Successfully installed helics-3.2.1.post8

(helics) C:\Users\schw197\OneDrive - PNNL\Documents\HELICS-Examples>helics --version
helics, version v3.2.1.post8

Python HELICS version v3.2.1.post8

HELICS Library version 3.2.1 (2022-06-16)

And now when I run the example (no change in code at all) there is one time step missing from the controller:

(helics) C:\Users\schw197\OneDrive - PNNL\Documents\HELICS-Examples\user_guide_examples\fundamental\fundamental_integration>helics run --path=fundamental_integration_runner.json  
[info] Running federation: fundamental_integration
[info] Running federate broker as a background process
[info] Running federate Charger as a background process
[info] Running federate Controller as a background process
[info] Running federate Battery as a background process
[info] Waiting for 4 processes to finish ...
[error] Process Controller has failed, killing other processes
[error] Process Charger exited with return code 1
[warn] Last 10 lines of Charger.log:
...
        No messages at endpoint Charger/EV4.soc recieved at time 604800.0
        Publishing charging voltage of 240  at time 604800.0
Sent message from endpoint Charger/EV4.soc to destination Controller/ep at time 604800.0 with payload SOC 0.777485
EV 5 time 604800.0
        Charging current: 3.46 from input Battery/EV5_current
         EV SOC estimate: 0.4261
        No messages at endpoint Charger/EV5.soc recieved at time 604800.0
        Publishing charging voltage of 240  at time 604800.0
Sent message from endpoint Charger/EV5.soc to destination Controller/ep at time 604800.0 with payload SOC 0.426108
Federate finalized
...
[error] Process Controller exited with return code 1
[warn] Last 10 lines of Controller.log:
...
Traceback (most recent call last):
  File "C:\Users\schw197\OneDrive - PNNL\Documents\HELICS-Examples\user_guide_examples\fundamental\fundamental_integration\Controller.py", line 171, in <module>
    axs[0].plot(xaxis, y[0], color='tab:blue', linestyle='-')
  File "C:\Users\schw197\Miniconda3\envs\helics\lib\site-packages\matplotlib\axes\_axes.py", line 1632, in plot
    lines = [*self._get_lines(*args, data=data, **kwargs)]
  File "C:\Users\schw197\Miniconda3\envs\helics\lib\site-packages\matplotlib\axes\_base.py", line 312, in __call__
    yield from self._plot_args(this, kwargs)
  File "C:\Users\schw197\Miniconda3\envs\helics\lib\site-packages\matplotlib\axes\_base.py", line 498, in _plot_args
    raise ValueError(f"x and y must have same first dimension, but "
ValueError: x and y must have same first dimension, but have shapes (672,) and (671,)
...
[error] Process Battery exited with return code 1
[warn] Last 10 lines of Battery.log:
...
        SOC: 0.6296
        Published Battery/EV4_current with value 2.46
Battery 5 time 604800.0
        Received voltage 240.00 from input Charger/EV5_voltage
        Effective R (ohms): 69.32
        Charging current (A): 3.46
        Added energy (kWh): 0.0138
        SOC: 0.4320
        Published Battery/EV5_current with value 3.46
Federate finalized
...
[info] Done.

(note the error in the controller.log)

Before I undo anything like setting the auto broker back, etc. I want to make sure that the pyhelics version is at least in a semi-stable state.

Thoughts?

[kdheepak]

    raise ValueError(f"x and y must have same first dimension, but "
ValueError: x and y must have same first dimension, but have shapes (672,) and (671,)

This appears to be a plotting issue? Maybe @trevorhardy has a better idea of what is going on.

I'm not exactly sure what is going on here but I can try on my end and see if I run into the same issue.

[eranschweitzer]

Yes it is a plotting issue, but the plotting issue is arising because in one run (with post6) there is an added timestep (604800.0) that the controller is processing.

In the successful run the log looks like this:

Requesting time 9223372036.854774
Granted time: 604800.0
Federate finalized

that is, once granted time is 604800.0 we have reached the end and the process ends.

Come to think of it, this might be the issue that I mentioned about the max time vs int(max_time) (see first comment above). Could this somehow be a double precision issue?

[kdheepak]

Could this somehow be a double precision issue?

Possibly. Pinging @trevorhardy and @phlptp since I don't think it is a pyhelics issue. It appears to be working for me on my MacOS.

$ git log -1 --pretty=format:"%H"
d4b43d17931669d4cbad2a994046c10bbc8626d0

$ 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-20.6.0:AppleClang-13.0.0.13000029",
    "cores": [
        "zmq",
        "zmqss",
        "tcp",
        "tcpss",
        "udp",
        "ipc",
        "interprocess",
        "inproc"
    ],
    "cpu": "Intel(R) Core(TM) i7-7920HQ CPU @ 3.10GHz",
    "cpucount": 8,
    "cputype": "x86_64",
    "memory": "16384 MB",
    "os": "Darwin  19.6.0  Darwin Kernel Version 19.6.0: Mon Apr 18 21:50:40 PDT 2022; root:xnu-6153.141.62~1/RELEASE_X86_64",
    "version": {
        "build": "",
        "major": 3,
        "minor": 2,
        "patch": 1,
        "string": "3.2.1 (2022-06-16)"
    },
    "zmqversion": "ZMQ v4.3.4"
}

[eranschweitzer]

Is the underlying helics build between post3 and post6 different? Otherwise, I don't see how it could be anything other than pyhelics, since that's the only thing I changed.

[eranschweitzer]

Wait, are you running it on my issue branch? Or on main? Screenshot seems like main.

If that's the case, please try running my branch.

[nightlark]

The wheel files for your platform could be downloaded from PyPI and unzipped, and then a diff/sha256sum of the binary HELICS related shared libraries could be done to check for differences.

On Friday, July 8, 2022, Eran @.***> wrote:

Is the underlying helics build between post3 and post6 different? Otherwise, I don't see how it could be anything other than pyhelics, since that's the only thing I changed.

— Reply to this email directly, view it on GitHub https://github.com/GMLC-TDC/HELICS-Examples/pull/72#issuecomment-1179334627, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6JBZ3LQS3F3JYGWASX4YLVTCE6NANCNFSM5264Q4CA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[eranschweitzer]

I embarrassingly have to admit that I don't understand what that all means

[kdheepak]

I ran your branch using post7, and it worked on my macos. I'm running it again with post8. I think the issue here is the HELICS_MAX_TIME. Using that as the final request time might be causing issues.

[eranschweitzer]

So the issue is that before I changed that I was getting the simulation hanging because the controller was terminating leaving the others in an infinite call loop.

[kdheepak]

It does seem to be working for me.

[eranschweitzer]

That's very strange. I'll play around here a bit more. This is probably really a numerical thing then where slight rounding in the wrong direction and we have an "off by one" type problem.

[eranschweitzer]

...so, now it's working for me too 🤷 (not changing anything) This is actually rather disconcerting that a simulation can succeed or not seemingly arbitrarily.

[kdheepak]

That’s what I thought was happening, and why I pinged @phlptp.

[kdheepak]

It is possible that using HELICS_MAX_TIME in this context is a “user error” and the examples should be updated.

[nightlark]

PyPI has a download page for every package: https://pypi.org/project/helics/#files

Wheels (.whl) files are “compiled” packages for a particular operating system (and CPU architecture, ARM or x86_64). They are really just zip files with .whl instead of .zip at the end of the file name. (Unzip one and explore to see what is inside, probably easier to check it out than me trying to explain it from memory on a phone).

sha256sum is a Linux/macOS command that calculates a hash for files. If any data in the file changes, the hash will be different. By comparing hashes for a set of files, you can check if any of them are identical or if they are all different.

On Friday, July 8, 2022, Eran @.***> wrote:

So the issue is that before I changed that I was getting the simulation hanging because the controller was terminating leaving the others in an infinite call loop.

— Reply to this email directly, view it on GitHub https://github.com/GMLC-TDC/HELICS-Examples/pull/72#issuecomment-1179341185, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6JBZYFQQTZHG4XF4RAMRDVTCGHDANCNFSM5264Q4CA . You are receiving this because you commented.Message ID: @.***>

[eranschweitzer]

It is possible that using HELICS_MAX_TIME in this context is a “user error” and the examples should be updated.

I was thinking that actually. There is really no reason to request HELICS_MAX_TIME rather than the end time of the simulation (at least non that I can see).

@trevorhardy thoughts on the matter? should I update/test that or leave for now?

[phlptp]

HELICS_MAX_TIME is a signal of cosimulation termination, if you request that time it usually means interrupt me if I get data or just run till everyone else ends.

[eranschweitzer]

HELICS_MAX_TIME is a signal of cosimulation termination, if you request that time it usually means interrupt me if I get data or just run till everyone else ends.

Understood, however, for whatever reason there seem to be some strange numerical issues with this variable.

[trevorhardy]

As long as HELICS_MAX_TIME is a valid constant that indicates the terminal time for co-simulation then its use should be supported as implemented here (if I'm remembering what I did correctly). If it is not working as correctly I'm assuming we would want to investigate first but may need to deprecate it in the future.

For now, its use is a well-established feature and I don't want to informally deprecate in a long-term way and tell users, "Yeah, it exists in the codebase but doesn't work correctly so don't use it." (In the short-term we may want to do that to keep things moving forward but that would only be after we put in some effort to figure out what's going on.)

[trevorhardy]

I've also recently had trouble with the User Guide examples passing and failing without any changes on my part. We've had users on Gitter reporting similar problems. Due to my crazy July (being out of office a lot), I haven't had time to dig into it but plan on doing so in August.

[eranschweitzer]

@trevorhardy, I'm not exactly sure what you mean with "HELICS_MAX_TIME is a valid constant that indicates the terminal time for co-simulation". HELICS_MAX_TIME is (as I understand) the largest number that HELICS can hold, not the termination time of the simulation. I just had the issue come up again for the fundamental_integration example,

• when I changed the requested time to HELICS_TIME_MAXTIME.
• I then changed it to int(HELICS_TIME_MAXTIME) and it worked.
• I then ran the example again, no change what-so-ever, and it didn't work.

I'm somewhat at a loss here. Can you tell me why/when the extra time request in destroy_federate was created. It wasn't in the fundamental integration case, so I added it back in. When I took it back out, things worked. 🤷

[trevorhardy]

@eranschweitzer, sorry for my sloppy language. As HELICS_MAX_TIME is the last time that a federate can request, it is the final or terminal time for all HELICS-based co-simulations. In the case of our User Guide examples, since all of them are requesting this time, it is thus becomes the literal last time step of the co-simulation.

destroy_federate contains all the good-housekeeping functionality that allows the federate to leave the federate in the approved manner. The time request in there is just to ensure that all signals that are destined for this federate are received by the federate. Under the assumption that most co-simulations send their last useful signals some time before HELICS_TIME_MAXTIME, requesting this time guarantees that those sent signals will be received by the federate. Specifically, it guarantees that the federate will exist for those signals to be routed to. We found that when this step wasn't done it was possible for a federate to leave the co-simulation before receiving any final signals. When this happened, the broker produces a warning message indicating that there were signals that couldn't be delivered because the federate no longer existed. Seeing these warning messages is disconcerting and may lead users (particularly new users) to believe something is wrong. Adding in this final time request allows all final signals to be received and not warnings to be generated.

What these examples are doing is not only valid from a HELICS standpoint but is also, arguably, preferable (as far as I can tell). If there is inconsistency you're seeing is caused by how the HELICS library and/or the Python language binding is interpreting HELICS_TIME_MAXTIME, I am asserting that it is a bug and not a user error. What the User Guide examples are doing in destroy_federate is vanilla use of the HELICS APIs and should consistently work.

[eranschweitzer]

Fair enough, then we definitely have an error 😃

Question Should I: a) Revert back to HELICS_TIME_MAXTIME everywhere and just add a note that this example may not work due to an outstanding bug (should I open an issue to link to?) b) keep the example as is so it works and open an issue to change it back once the bug is fixed?

Just, FYI, in the examples it is not the case that all federates request HELICS_TIME_MAXTIME. the Battery and Charger are always requesting whatever the last time is plus a fixed delta. Only the controller is requesting HELICS_TIME_MAXTIME. The problem that is arising is precisely the one you describe, namely, it appears that the controller is waiting on responses from Battery and Charger federates, but those have already exited the federation. Rather than an error message in the broker log though, the simulation just hangs indefinitely.

[trevorhardy]

@eranschweitzer, what is the band-aid fix you're proposing exactly? If you've got something that works consistently and is not laborious to implement, I would propose doing so until we figure out what's going on. Working examples are a big part of the HELICS education experience.

When I looked at the "advanced default" and "fundamental integration" examples Battery.py in both have the "destroy_federate" function with the time request for HELICS_TIME_MAXTIME. I'm not following your "FYI" comment from your previous post. It looks like all the federates request HELICS_TIME_MAXTIME before existing the federation. Am I missing something?

[eranschweitzer]

The FYI comment is about the normal time request, not during the federate destruction. So, in the while grantedtime < total_time loop. There only the controller requests MAX TIME.

My "band-aid" fix was to change that so the controller requests total_time. Though, I have to admit that I didn't try running it multiple times to see if for some reason it would there too inexplicably start hanging.

Finally, the last time request was not in the integration example initially. Should I add it in there?

[trevorhardy]

@eranschweitzer, the final time request is in the code for all three federates in the "main" branch; are you not seeing them in your code base?

Battery.py: destroy_federate and destroy_federate call

Charger.py: destroy_federate and destroy_federate call

Controller.py: destroy_federate and destroy_federate call

[eranschweitzer]

I'm seeing that. What I meant was that when I changed the following calls in the controller federate I stopped having the issues: https://github.com/GMLC-TDC/HELICS-Examples/blob/3a4df3d8a72c098ffad4daa3b3f7a41e5f28b39b/user_guide_examples/advanced/advanced_default/Controller.py#L81-L84

https://github.com/GMLC-TDC/HELICS-Examples/blob/3a4df3d8a72c098ffad4daa3b3f7a41e5f28b39b/user_guide_examples/advanced/advanced_default/Controller.py#L137-L139

(and similarly for the integration example)

[eranschweitzer]

Also, here is the destroy federate code you linked to for the controller. Notice that there is no final time request: https://github.com/GMLC-TDC/HELICS-Examples/blob/d4b43d17931669d4cbad2a994046c10bbc8626d0/user_guide_examples/fundamental/fundamental_integration/Controller.py#L26-L41

As opposed to the call you linked to for the Charger: https://github.com/GMLC-TDC/HELICS-Examples/blob/d4b43d17931669d4cbad2a994046c10bbc8626d0/user_guide_examples/fundamental/fundamental_integration/Charger.py#L36-L54

Or the controller in the advanced default example: https://github.com/GMLC-TDC/HELICS-Examples/blob/d4b43d17931669d4cbad2a994046c10bbc8626d0/user_guide_examples/advanced/advanced_default/Controller.py#L27-L45

[trevorhardy]

Ahh, I see.

Do a few iterations in each case to make sure the existing method is truly broken and requesting the simulation terminal time fixes it. Assuming that holds up, make the change, push it into the branch where you're making the update and file a bug related to the problems with HELICS_TIME_MAXTIME.

[trevorhardy]

Unrelatedly, was auto-broker truly not working for you? I know it was working for me previously and if its broken for you now we need to file another bug on that as well.

[kdheepak]

The auto broker functionality was being ported to the pyhelics repo around the time this PR was being made. So there was some confusion associated with that.

FWIW, GMLC-TDC/helics-cli is archived and deprecated, and future functionality will be part of GMLV-TDC/pyhelics which comes as part of the pip install helics. The “auto broker” functionality is available in the latest releases.

[trevorhardy]

@kdheepak, has that deprecation already taken place? If so, I have documentation I need to update.

[kdheepak]

Yes, it has happened and almost all the features from helics-cli have been ported to pyhelics. I've sent you messages on teams with more information, but I guess you are still traveling so haven't had a chance to follow up on that.

[trevorhardy]

Got it. Yeah, still getting my head around all that I missed while out.

I know that our installation guide page will need to change and there may be other updates to the documentation. These will be commits that we need in the HELICS library as a hotfix to the documentation so it can go live ASAP.

[phlptp]

Submit doc only hot fix prs directly to main

We will merge back into develop when near time for a release.

---

Sent from Workspace ONE Boxerhttps://whatisworkspaceone.com/boxer

On July 19, 2022 at 11:22:04 AM MDT, Trevor Hardy @.***> wrote:

Got it. Yeah, still getting my head around all that I missed while out.

I know that our installation guide page will need to change and there may be other updates to the documentation. These will be commits that we need in the HELICS library as a hotfix to the documentation so it can go live ASAP.

— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/GMLC-TDC/HELICS-Examples/pull/72*issuecomment-1189357537__;Iw!!G2kpM7uM-TzIFchu!lQocMpJJvwYX6BNghYZPENA22xvjcltZA0jAKXIeyHOBvEwi21R9CGV0zhOstic$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/AE5VWELRTEFXSSDW44LAE3LVU3P3PANCNFSM5264Q4CA__;!!G2kpM7uM-TzIFchu!lQocMpJJvwYX6BNghYZPENA22xvjcltZA0jAKXIeyHOBvEwi21R9CGV0VPzpuBg$. You are receiving this because you were mentioned.Message ID: @.***>

[trevorhardy]

@eranschweitzer, in reading through the extensive comments it looks like we are waiting on a bug fix related to MAXTIME from @phlptp; is that your understanding?

[eranschweitzer]

It's possible I dropped the ball here. Let me try running address your comments here:

Ahh, I see.

Do a few iterations in each case to make sure the existing method is truly broken and requesting the simulation terminal time fixes it. Assuming that holds up, make the change, push it into the branch where you're making the update and file a bug related to the problems with HELICS_TIME_MAXTIME.

Before I fully answer that question.

[eranschweitzer]

Update to fundamental integration

Controller.py requested time	helicsFederateRequestTime call in destroy_federate	Observed Outcome
total_interval	NO	Successful Termination (multiple times) on one run though Controller successfully finalized, but Battery and Charger hang on last timestep ❌
total_interval	YES	Simulation hangs on the last timestep ❌
HELICS_TIME_MAXTIME	YES	Simulation hangs on the last timestep ❌
HELICS_TIME_MAXTIME	NO	Controller successfully finalized, but Battery and Charger hang on last timestep ❌
int(HELICS_TIME_MAXTIME)	NO	Successful Termination on one run. On another Controller successfully finalized, but Battery and Charger hang on last timestep ❌
int(HELICS_TIME_MAXTIME)	YES	Successful Termination (multiple times). On one run though simulation hangs on last timestep ❌

Update to advanced default

Controller.py requested time	helicsFederateRequestTime call in destroy_federate	Observed Outcome
total_interval	NO	Successful Termination (multiple times) ✅
total_interval	YES	Successful Termination (multiple times) ✅
HELICS_TIME_MAXTIME	YES	Successful Termination (multiple times) ✅
HELICS_TIME_MAXTIME	NO	Successful Termination (multiple times) ✅

Summary

There is definitely something weird going on, but I'm starting to think it might be with they PyHELICS binding of HELICS_TIME_MAXTIME 🤔

The Advanced Default example is robust to the little experimentation demonstrated, while the fundamental integration example seems to be very sensitive to what should be meaningless changes.

@trevorhardy how would you like to proceed here?

[trevorhardy]

@eranschweitzer, if you're sure this is a a pyhelics bug then I think we can report it in that repo. If you're not sure we should sit down and talk it over just so I can make sure I understand all the details. I doubt I'll have a better grasp of the root problem but it will be good for me to be up-to-speed on this.