Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repetitive query of coil.amplitude freezes the process #7

Open
HUtangge opened this issue Mar 5, 2020 · 17 comments
Open

Repetitive query of coil.amplitude freezes the process #7

HUtangge opened this issue Mar 5, 2020 · 17 comments
Assignees

Comments

@HUtangge
Copy link

HUtangge commented Mar 5, 2020

If I call coil.amplitude for several times without changing the amplitude on the device, it will freeze.
https://github.com/pyreiz/ctrl-localite/blob/master/localite/coil.py#L143

@agricolab
Copy link
Member

agricolab commented Mar 5, 2020

Please add a minimal script to reproduce the error.

Generally, this likely occurs if you request the amplitudes too often and too fast. Localite 4.0 answers requests asychronously, and we check on our side that we received an answer for every query. If you query too often too fast, this pipeline can get clogged. This can not be fixed.

@agricolab agricolab changed the title coil amplitude does not work well Repetitive query of coil.amplitude freezes the process Mar 5, 2020
@moan0s
Copy link
Contributor

moan0s commented Mar 9, 2020

import time
from localite.api import Coil
c = Coil(0)
while True:
    c.amplitude 
    time.sleep(.75)

Works for >= 0.8 seconds

@agricolab
Copy link
Member

Could be related to the minimal latency between consecutive messages sent by Localite4. Should be settable in the Preference Editor (quote from email):

  • Button "Init Missing Data" / "Fehlende Daten initialisieren" anklicken
  • Button "Edit Configuration..." / "Konfiguration bearbeiten..." anklicken.
  • Knoten "External Communication" auswählen und beim Schlüssel/Key "Minimum Position Transfer Interval" den gewünschten Wert in Millisekunden eintragen.

@moan0s
Copy link
Contributor

moan0s commented Mar 9, 2020

Related: The function coil.amplitude = stimulation_intensity does not return every ~30 calls

PUSH: loc {"coil_0_amplitude": 52} @ 14442.99697
PUSH: loc {"get": "coil_0_amplitude"} @ 14442.99753

A manual change of amplitude on the MagVenture lets the program continue.
The fix

@amplitude.setter
def amplitude(self, amplitude: int) -> int:
    "get the current amplitude in MSO%"
    msg = f'{{"coil_{self._id}_amplitude": {amplitude}}}'
    self._push_loc(msg=msg)
    sleep(1.5)
    return self.request("amplitude")

seems to solve the problem (0.8 seconds was not enough)

@agricolab
Copy link
Member

It is probably better to remove the request at the end , i.e. return self.request("amplitude") instead of adding the sleep. we can check in the getter whether 1.5s have passed since last request or return a cached answer.

Rationale is that we do not want to block an experiment for 1.5 seconds everytime we set a new amplitude.

agricolab added a commit that referenced this issue Mar 9, 2020
@agricolab
Copy link
Member

Can you test whether this solved the issue?

@moan0s
Copy link
Contributor

moan0s commented Mar 10, 2020

Still has problems, probably even more severly, sometimes after the first request. Script with test:

import time
from localite.api import Coil
c = Coil(0)
time.sleep(1)

for i in range(1,100,1):
    c.amplitude = i
    time.sleep(1)

@HUtangge
Copy link
Author

During hotspot search, if we want to get the information of coil.amplitude continuously, then we would call amplitude = coil.amplitude. This still have problem, scripts for testing:

from localite.coil import Coil
from time import sleep
coil = Coil(0)
for i in range(1,100,1):
    amplitude = coil.amplitude
    coil.trigger()

@ethanrich
Copy link

ethanrich commented May 11, 2020

This issue is persisting in East measure as well and seems to remain unresolved here for West. Currently, it is holding back the Breathing-FES study from starting.

When running any form of:

while True:
   coil.amplitude

such as that found here with the amplitude request in line 201, either the function await_response() or get_as_list() (from localite flow ) enter an infinite loop.

In order to prevent this issue with the coming studies, the RMT and HS search scripts (found here) are being adapted to remove coil.amplitude requesting within a while loop. I will comment again and close this issue once this has been implemented.

@ethanrich
Copy link

This issue is persisting despite removing coil.amplitude from a loop. As of today, an error is thrown
here when content, which should have held the coil amplitude, is returned as NoneType as it is not interable.

@agricolab
Copy link
Member

agricolab commented May 11, 2020

Thanks for the analysis!

Very challenging to debug. First of all, the old scripts run fine when the old localite-client was still being used. Therefore, this could be linked to how we deal with Localite missing some of our queries (in former times, we just ignored it, so examiners had to repeat triggering etc.) Now, we expect for every query an answer and the package automatically retries the query if there is no answer in time (see here ). Yet, i think it should only do so for queries containing target_index (see here). Anyways, gist is, this might be linked to the way the Localite-Server is handling messages, and if it never gives the right answer, the Client loops indefinitely. Yet, why does this happen in a while loop?

I can offer two additional speculations. The first is: This is related to repetitive queries to the ethernet stack, especially something intrinsic to Windows 10 OS. This could be tested by running the same code from a Linux machine and see whether the behaviour differs. An alternative (or maybe even linked) explanation is how a while loop is being compiled by CPython. The while loop is being compiled to bytecode before it is run. Consider that coil.amplitude is a property. Let"s assume that when compiling a while-loop, repetitive queries to an object's property are being optimized. Then, the code is compiled to different bytecode than when compiling the same statement outside of a while-loop. One option to find out whether that's the case is inspecting the bytecode instructions with dis.dis and see whether the respective parts differ. Alternatively, we could also try to see whether this still occurs when we would inline the coil.amplitude-queries with e.g. self.request("amplitude") or even more low-level code, or when we wrap the repeated actions in a function.

@ethanrich
Copy link

Hi Robert, thanks for the reply . I Just have a few questions:

So far, dis.dis(coil.amplitude) returns TypeError: don't know how to disassemble int objects. Am I missing how to apply dis.dis to a property?

What do you mean by inlining queries? The same error occurs for coil.request('amplitude').

@agricolab
Copy link
Member

agricolab commented May 11, 2020

What do you mean by inlining queries? The same error occurs for coil.request('amplitude').

This. A pity the error still occurs.

So far, dis.dis(coil.amplitude) returns TypeError: don't know how to disassemble int objects. Am I missing how to apply dis.dis to a property?

Probably not. This likely just shows that properties are tricky when turned into bytecode.

Considering the error occurs also when the property was inlined, it is likely not due to bytecode, but some other issue with the ethernet and the client/server relationship

@ethanrich
Copy link

Thanks for the guidance, I'll continue on this direction.

@agricolab agricolab self-assigned this Jul 7, 2020
@agricolab
Copy link
Member

hey @ethanrich , any update on the status of this issue?

@ethanrich
Copy link

ethanrich commented Oct 21, 2021

I failed to solve the connection issue. Ultimately, I reverted to using the Localite Client class from the old branch.

@moan0s
Copy link
Contributor

moan0s commented Jan 24, 2022

It is possible to unjam the system by starting a new instance of Coil in a seperate thread. maybe it is possible to find our what is done when starting the thread so that the old thread also runs again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants