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

WIP: ENH: Make pyAFQ tests faster, add export all #534

Merged
merged 5 commits into from Mar 23, 2023

Conversation

36000
Copy link
Contributor

@36000 36000 commented Mar 13, 2023

Closes #422 and #434 by adding in the export all and choice of export functionality for pyAFQ.

I also want to reduce the runtime of the tests here. @mattcieslak , is there a way to change the parameters for one of the nodes in the json file for the test only? For example, I would like select here to be 1e4 instead of 1e6, but only when doing the circle CI tests:

@mattcieslak
Copy link
Collaborator

I'd recommend adding a few lines in here so that when the --sloppy flag is used the parameters get changed to something lower.

@mattcieslak
Copy link
Collaborator

@36000 I split this into two tests, one where pyafq does everything and another where the tractography is done in mrtrix beforehand. It seems like something in AFQ itself is making the test take a long time. Can you think of any shortcuts we might be able to change in the config when the --sloppy flag gets used?

@arokem
Copy link
Contributor

arokem commented Mar 22, 2023

Could we potentially configure it to only find some bundles? That would shorten it substantially, I would think.

Another option would be to skip the visualizations, which can also take some time.

@mattcieslak
Copy link
Collaborator

I like the "only a few bundles" idea. That way we can test a few of them all the way through, including visualization. Is there a spot in the json where you can specify which bundles to do?

@36000
Copy link
Contributor Author

36000 commented Mar 23, 2023

I changed it to do only a few bundles, hopefully also have a much faster registration, and generate only 1 visualization, so that the visualization library is tested but we don't spend too much time in it. Let's see if this test passes and if needs to be sped up more in other places.

@mattcieslak
Copy link
Collaborator

Down to ~30 minutes! Thank you @36000! Does this test everything you'd like to test still? If so I'm going to merge

@mattcieslak mattcieslak merged commit 08e7925 into PennLINC:master Mar 23, 2023
2 checks passed
@mattcieslak mattcieslak self-assigned this Apr 17, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants