search

nengo / nengo-loihi

Run Nengo models on Intel's Loihi chip
https://www.nengo.ai/nengo-loihi/
Other
35 stars 12 forks source link

Fix travis CI #218

Closed arvoelke closed 5 years ago

arvoelke commented 5 years ago

The first commit makes our pytest.raises checks more strict. This is something I was using to debug, and isn't strictly needed by this PR, but IMO is a good thing to do.

The second commit localizes the os.chdir to be within a try-finally block. This avoids a really weird issue we were seeing as a part of #217 (e.g., see this job and look for nengo_loihi/hardware/tests/test_allocators.py::test_block_size).

Essentially... if an error happens when the model is being built, then the current working directory (cwd) is permanently changed. More specifically, an error occurs within HardwareInterface.__init__, and so __exit__ is never invoked. As a result, the directory is never changed back. This only occurs for --target loihi, and causes coverage.xml to be written to the wrong directory. Part of what made this issue weird is that coverage.xml is only written to the wrong directory if -n is omitted, but not for -n 1. This may be a subtle aspect of how pytest spins up its workers.

The second commit also adds a check to pytest's setup/teardown to make sure the directory is not changing. This test would fail on test_block_size with --target loihi if it were not for the fix.

Note this does not solve #217 (i.e., it does not make nengo-loihi process safe). The issue explained in the commit message of e9cd851b01b20b774d58c82a2ba775b531cf2bab is still present. Solving this is outside the scope of PR.

hunse commented 5 years ago

I think we should be doing the chdir in create_io_snip, which gets called when run is called, not when the HardwareInterface is created. It has the added advantage of keeping all the snip stuff in one place. I don't think there's any need to change the directory in __init__.

arvoelke commented 5 years ago

Wait why do we even chdir at all? It must have been necessary at some point, but that no longer seems to be the case. Everything passes with nxsdk-0.8.0 without the chdir. I added a couple commits to demonstrate.

hunse commented 5 years ago

Yeah, I thought I remembered Intel trying to fix that, though I don't ever remember them conclusively saying they did. But if it works without it for 0.8, then let's just get rid of it. We can probably drop support for pre-0.8 by this point. It was definitely necessary with some earlier version.