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

Performance improvement for CoTeDe #160

Merged
merged 8 commits into from
Jun 1, 2016
Merged

Performance improvement for CoTeDe #160

merged 8 commits into from
Jun 1, 2016

Conversation

castelao
Copy link
Member

@castelao castelao commented Apr 9, 2016

Before it would call CoTeDe to run a full set of tests, like the full GTSPP recommendation, to extract only one test at a time. Now it calls CoTeDe to run only the specific test of interest.

It should make significant performance difference for most tests using CoTeDe. Since AutoQC runs in parallel, and only a portion of tests use CoTeDe, it requires a large data processing to identify this time difference.

castelao added 8 commits April 8, 2016 10:14
DummyCNV was moved into wodpy as Wod4CoTeDe with few improvements.
Adding .datetime(), .z_level_qc() & .t_level_qc()
Before it was always running a full block of tests, to get the results
of a single test
CoTeDe now try to run the minimum ammount of tests required, so it is
not required anymore to explicitly define setup with a dictionary.
There was a conceptual error before. It should be faster now.
@castelao
Copy link
Member Author

@s-good and @BillMills , are you OK with this PR?

It says that there is a conflict, but I can't see what it is.

@bkatiemills
Copy link
Member

@castelao if you on your local machine switch to your cotedeupdate branch, and then do git pull upstream master (assuming you have a remote called upstream pointed at this repo), git should report all the files with merge conflicts in them, and annotate them inline. Let me know if this works for you!

@castelao
Copy link
Member Author

Hi @BillMills . Yes, I called upstream, and yes I usually keep them sync, like I did for the previous PR deac0a8. I guess these PRs took so long that i missed the opportunity to sync again. I'll work on that when I have chance.

Thanks,

@bkatiemills
Copy link
Member

Hey @castelao - any luck here? Once #161 wraps up we will be ready for 1.0, and I really want this PR included there; let me know if I can help at all!

@bkatiemills bkatiemills mentioned this pull request Jun 1, 2016
@bkatiemills bkatiemills merged commit cf9144e into IQuOD:master Jun 1, 2016
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.

2 participants