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.

Sign up for GitHub

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

Jump to bottom

Speed up tests (swc, plotting) #479

Merged
merged 3 commits into from
Nov 14, 2024
Merged

Speed up tests (swc, plotting) #479

merged 3 commits into from
Nov 14, 2024

Conversation

fabioseel
Copy link
Contributor

Main contributions:

  • swc tests now significantly faster (change to sparse solver)
  • plotting tests a lot faster

minor contributions:

  • added test fixtures for compartment, cell, morph_cell, branch and network -> not sure how much it helps / improves readability here. Could also revert these changes. (might be a first step for Refactor tests #449)

jnsbck
jnsbck approved these changes Nov 10, 2024
View reviewed changes
Copy link
Contributor

@jnsbck jnsbck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a lot to critique here. Left only a couple comments. Feel free to merge this once adressed. :)

tests/test_plotting_api.py
dirname = os.path.dirname(__file__)
fname = os.path.join(dirname, "swc_files", "morph.swc")
cell = jx.read_swc(fname, nseg=4)
@pytest.fixture(scope="module")
Copy link
Contributor

@jnsbck jnsbck Nov 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! I think we could even move them to a separate file, to make them available to all other tests. tests/conftest.py.

Copy link
Contributor

@jnsbck jnsbck Nov 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to replace as many of the other tests that rely on comp or net etc. with these fixtures. But also fine if you don't and then just leave #449 open and just add a quick comment to it.

EDIT: I have implemented something like this in #499 :)

michaeldeistler
michaeldeistler approved these changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@michaeldeistler michaeldeistler left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Good to go also from my side! @fabioseel let us know if/when we can merge it! Thanks a ton!

@jnsbck jnsbck mentioned this pull request Nov 14, 2024
Merged
@fabioseel
Copy link
Contributor Author

as Jonas is addressing the fixture issue you can merge it 👍

@jnsbck jnsbck merged commit 74f5994 into jaxleyverse:main Nov 14, 2024
1 check passed
@jnsbck
Copy link
Contributor

jnsbck commented Nov 14, 2024

Awesome, thanks a ton!

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

@jnsbck jnsbck jnsbck approved these changes

@michaeldeistler michaeldeistler michaeldeistler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Speed up plotting (reduce mesh resolution)
3 participants
@fabioseel @jnsbck @michaeldeistler