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

Use filenames that are guaranteed to be unique for standalone #455

Merged
merged 3 commits into from Apr 22, 2015

Conversation

Projects
None yet
3 participants
@mstimberg
Member

mstimberg commented Apr 21, 2015

Arrays were previously stored in the results directory using the name of the array as the filename. On file systems that are not case sensitive (e.g. Windows), this is not guaranteed to work. The most common situation were this leads to problems is with a variable "I" in a neuron group, since it also contains the variable "i" by default. As an easy fix, the filename now uses the name of the variable plus its hash value. Note that users should never have to construct such file names manually, if for any reasons they cannot access the values via the normal state variable mechanism, they should use device.get_array_filename to get the filename.

mstimberg added some commits Apr 21, 2015

Use filenames that are guaranteed to be unique for standalone
Arrays were previously stored in the results directory using the name of the array as the filename. On file systems that are not case sensitive (e.g. Windows), this is not guaranteed to work. The most common situation were this leads to problems is with a variable "I" in a neuron group, since it also contains the variable "i" by default. As an easy fix, the filename now uses the name of the variable plus its hash value. Note that users should never have to construct such file names manually, if for any reasons they cannot access the values via the normal state variable mechanism, they should use `device.get_array_filename` to get the filename.
Only run long tests if explicitly requested, also for standalone/stan…
…dalone-compatible

Otherwise tests may reach the time out on travis/appveyor
@mstimberg

This comment has been minimized.

Show comment
Hide comment
@mstimberg

mstimberg Apr 21, 2015

Member

Note that only the test suite of the first commit will run the tests that previously failed (but it may reach a time out after that). The second commit restricts the test suite to tests that are not marked as long, also for the standalone-compatible tests (it was previously ignored for all the standalone tests)

Member

mstimberg commented Apr 21, 2015

Note that only the test suite of the first commit will run the tests that previously failed (but it may reach a time out after that). The second commit restricts the test suite to tests that are not marked as long, also for the standalone-compatible tests (it was previously ignored for all the standalone tests)

@mstimberg

This comment has been minimized.

Show comment
Hide comment
@mstimberg

mstimberg Apr 21, 2015

Member

Ah, damnit, that wasn't smart, appveyor canceled the test on the previous commit...

Member

mstimberg commented Apr 21, 2015

Ah, damnit, that wasn't smart, appveyor canceled the test on the previous commit...

@mstimberg

This comment has been minimized.

Show comment
Hide comment
@mstimberg

mstimberg Apr 21, 2015

Member

Maybe run the long tests manually on your machine? If it runs fine on travis with the "hashed" filenames, it should run fine on appveyor as well, so I'm not too worried.

Member

mstimberg commented Apr 21, 2015

Maybe run the long tests manually on your machine? If it runs fine on travis with the "hashed" filenames, it should run fine on appveyor as well, so I'm not too worried.

@thesamovar

This comment has been minimized.

Show comment
Hide comment
@thesamovar

thesamovar Apr 21, 2015

Member

I'm getting most tests failing with errors like this.

======================================================================
ERROR: Test the correct handling of referred scalar variables in subexpressions
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Anaconda\lib\site-packages\nose\case.py", line 197, in runTest
    self.test(*self.arg)
  File "C:\programming\brian2\brian2\tests\test_neurongroup.py", line 127, in test_referred_scalar_variable
    assert_allclose(G2.out[:], np.arange(10)+1)
  File "C:\programming\brian2\brian2\core\variables.py", line 856, in __getitem__
    return self.get_item(item, level=1)
  File "C:\programming\brian2\brian2\core\variables.py", line 846, in get_item
    run_namespace=namespace)
  File "C:\programming\brian2\brian2\core\base.py", line 249, in device_override_decorated_function
    return getattr(curdev, name)(*args, **kwds)
  File "C:\programming\brian2\brian2\devices\cpp_standalone\device.py", line 384, in variableview_get_subexpression_with_index_array
    run_namespace=run_namespace)
  File "C:\programming\brian2\brian2\core\base.py", line 251, in device_override_decorated_function
    return func(*args, **kwds)
  File "C:\programming\brian2\brian2\core\variables.py", line 1195, in get_subexpression_with_index_array
    codeobj_class=get_default_codeobject_class('codegen.string_expression_target')
  File "C:\programming\brian2\brian2\codegen\codeobject.py", line 339, in create_runner_codeobj
    override_conditional_write=override_conditional_write,
  File "C:\programming\brian2\brian2\devices\device.py", line 264, in code_object
    name=name)
  File "C:\programming\brian2\brian2\codegen\runtime\numpy_rt\numpy_rt.py", line 52, in __init__
    self.variables_to_namespace()
  File "C:\programming\brian2\brian2\codegen\runtime\numpy_rt\numpy_rt.py", line 75, in variables_to_namespace
    value = var.get_value()
  File "C:\programming\brian2\brian2\core\variables.py", line 510, in get_value
    return self.device.get_value(self)
  File "C:\programming\brian2\brian2\devices\cpp_standalone\device.py", line 348, in get_value
    with open(fname, 'rb') as f:
IOError: [Errno 2] No such file or directory: 'c:\\users\\dgoodman\\appdata\\local\\temp\\tmpiqd9ya\\results\\_array_neurongroup_1_freq_171961427'
Member

thesamovar commented Apr 21, 2015

I'm getting most tests failing with errors like this.

======================================================================
ERROR: Test the correct handling of referred scalar variables in subexpressions
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Anaconda\lib\site-packages\nose\case.py", line 197, in runTest
    self.test(*self.arg)
  File "C:\programming\brian2\brian2\tests\test_neurongroup.py", line 127, in test_referred_scalar_variable
    assert_allclose(G2.out[:], np.arange(10)+1)
  File "C:\programming\brian2\brian2\core\variables.py", line 856, in __getitem__
    return self.get_item(item, level=1)
  File "C:\programming\brian2\brian2\core\variables.py", line 846, in get_item
    run_namespace=namespace)
  File "C:\programming\brian2\brian2\core\base.py", line 249, in device_override_decorated_function
    return getattr(curdev, name)(*args, **kwds)
  File "C:\programming\brian2\brian2\devices\cpp_standalone\device.py", line 384, in variableview_get_subexpression_with_index_array
    run_namespace=run_namespace)
  File "C:\programming\brian2\brian2\core\base.py", line 251, in device_override_decorated_function
    return func(*args, **kwds)
  File "C:\programming\brian2\brian2\core\variables.py", line 1195, in get_subexpression_with_index_array
    codeobj_class=get_default_codeobject_class('codegen.string_expression_target')
  File "C:\programming\brian2\brian2\codegen\codeobject.py", line 339, in create_runner_codeobj
    override_conditional_write=override_conditional_write,
  File "C:\programming\brian2\brian2\devices\device.py", line 264, in code_object
    name=name)
  File "C:\programming\brian2\brian2\codegen\runtime\numpy_rt\numpy_rt.py", line 52, in __init__
    self.variables_to_namespace()
  File "C:\programming\brian2\brian2\codegen\runtime\numpy_rt\numpy_rt.py", line 75, in variables_to_namespace
    value = var.get_value()
  File "C:\programming\brian2\brian2\core\variables.py", line 510, in get_value
    return self.device.get_value(self)
  File "C:\programming\brian2\brian2\devices\cpp_standalone\device.py", line 348, in get_value
    with open(fname, 'rb') as f:
IOError: [Errno 2] No such file or directory: 'c:\\users\\dgoodman\\appdata\\local\\temp\\tmpiqd9ya\\results\\_array_neurongroup_1_freq_171961427'
@thesamovar

This comment has been minimized.

Show comment
Hide comment
@thesamovar

thesamovar Apr 21, 2015

Member

The issue was backslashes and escapes. Windows filenames have backwards slash rather than forward slash, and when pasted directly into C++ code these were being interpreted as escape sequences. I modified the templates to handle this issue (because the get_array_filename method ought to return a value without the slashes escaped).

Member

thesamovar commented Apr 21, 2015

The issue was backslashes and escapes. Windows filenames have backwards slash rather than forward slash, and when pasted directly into C++ code these were being interpreted as escape sequences. I modified the templates to handle this issue (because the get_array_filename method ought to return a value without the slashes escaped).

@thesamovar

This comment has been minimized.

Show comment
Hide comment
@thesamovar

thesamovar Apr 21, 2015

Member

Oops, that last commit messed up that file because apparently my git configuration on this machine is not correctly set up to handle CRLF, so it shows every line as having changed.

Member

thesamovar commented Apr 21, 2015

Oops, that last commit messed up that file because apparently my git configuration on this machine is not correctly set up to handle CRLF, so it shows every line as having changed.

@mstimberg

This comment has been minimized.

Show comment
Hide comment
@mstimberg

mstimberg Apr 21, 2015

Member

Hmm, indeed, but now the file is actually using the recommended file ending (LF). If you check it out on a machine with core.autocrlf you should get it again with CRLF line endings. Which reminds me, should we maybe do the big clean-up commit for the line endings soon?

Member

mstimberg commented Apr 21, 2015

Hmm, indeed, but now the file is actually using the recommended file ending (LF). If you check it out on a machine with core.autocrlf you should get it again with CRLF line endings. Which reminds me, should we maybe do the big clean-up commit for the line endings soon?

@thesamovar

This comment has been minimized.

Show comment
Hide comment
@thesamovar

thesamovar Apr 21, 2015

Member

Tests passed on my computer and apparently on appveyor, so I guess this is probably going to be good. Happy for it to be merged once they all report back.

Member

thesamovar commented Apr 21, 2015

Tests passed on my computer and apparently on appveyor, so I guess this is probably going to be good. Happy for it to be merged once they all report back.

@thesamovar

This comment has been minimized.

Show comment
Hide comment
@thesamovar

thesamovar Apr 21, 2015

Member

Oh and for the line endings, yeah now would be a good time I guess. Is the idea to have everything with just LF and we make sure to have core.autocrlf on to handle that?

Member

thesamovar commented Apr 21, 2015

Oh and for the line endings, yeah now would be a good time I guess. Is the idea to have everything with just LF and we make sure to have core.autocrlf on to handle that?

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Apr 22, 2015

Coverage Status

Coverage decreased (-0.01%) to 87.52% when pulling b34fbd3 on standalone_filenames into 1307970 on master.

coveralls commented Apr 22, 2015

Coverage Status

Coverage decreased (-0.01%) to 87.52% when pulling b34fbd3 on standalone_filenames into 1307970 on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Apr 22, 2015

Coverage Status

Coverage decreased (-0.01%) to 87.52% when pulling b34fbd3 on standalone_filenames into 1307970 on master.

coveralls commented Apr 22, 2015

Coverage Status

Coverage decreased (-0.01%) to 87.52% when pulling b34fbd3 on standalone_filenames into 1307970 on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Apr 22, 2015

Coverage Status

Coverage decreased (-0.01%) to 87.52% when pulling b34fbd3 on standalone_filenames into 1307970 on master.

coveralls commented Apr 22, 2015

Coverage Status

Coverage decreased (-0.01%) to 87.52% when pulling b34fbd3 on standalone_filenames into 1307970 on master.

@mstimberg

This comment has been minimized.

Show comment
Hide comment
@mstimberg

mstimberg Apr 22, 2015

Member

All tests passed, merging now.

Member

mstimberg commented Apr 22, 2015

All tests passed, merging now.

mstimberg added a commit that referenced this pull request Apr 22, 2015

Merge pull request #455 from brian-team/standalone_filenames
Use filenames that are guaranteed to be unique for standalone

@mstimberg mstimberg merged commit 9c18514 into master Apr 22, 2015

3 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@mstimberg mstimberg removed the in progress label Apr 22, 2015

@mstimberg mstimberg deleted the standalone_filenames branch Apr 22, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment