-
Notifications
You must be signed in to change notification settings - Fork 23
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
redo internals of State #185
base: main
Are you sure you want to change the base?
Conversation
Having now implemented this for analysis, I get the following results:
|
Though the tests pass, this might have unexpected effects in various places because analyses with the same weight will now be displayed in alphabetical order rather than in an unspecified order strongly influenced by the order they appear in the dictionary. I can probably restore the older order, but I'm not sure whether that matters. |
Restoring the old ordering is just diff --git a/lttoolbox/reusable_state.cc b/lttoolbox/reusable_state.cc
index aa55c95..1126d71 100644
--- a/lttoolbox/reusable_state.cc
+++ b/lttoolbox/reusable_state.cc
@@ -241,11 +241,17 @@ void ReusableState::extract(size_t pos, UString& result, double& weight,
}
}
+bool comp_pair(const std::pair<double, UString>& a,
+ const std::pair<double, UString>& b)
+{
+ return a.first < b.first;
+}
+
void NFinals(std::vector<std::pair<double, UString>>& results,
size_t maxAnalyses, size_t maxWeightClasses)
{
if (results.empty()) return;
- sort(results.begin(), results.end());
+ sort(results.begin(), results.end(), comp_pair);
if (maxAnalyses < results.size()) {
results.erase(results.begin()+maxAnalyses, results.end());
} which has a negligible impact on the runtime. I leave the question open to bikeshedding. |
Having tried block sizes of |
dd5899d
to
a67d1e0
Compare
I restored the original output ordering. I haven't applied it everywhere it could be applied, though that's largely because biltrans is structured in a way that makes it awkward to incorporate and the other modes have no tests. |
also tweak capitalization handling in generator
As it stands now, this PR has subtly different capitalization output for generation, so I'll revert those parts.
|
turns out I actually just forgot to check the dirty bit |
Unfortunately, the memory usage on this explodes for nno-nob, probably due to regexes (see #167). |
This PR implements and alternative to
State
that builds a graph of output paths rather than copying a collection of vectors. The result is thatstep()
is faster because it's not doing any allocations andfilterFinals()
is slower because it has to collect the path rather than just iterating over an array. Initial experimentation suggests this is an overall speedup, but the constants inresuable_state.h
should probably be tuned empirically.I currently have it as a separate class rather than directly modifying
State
in case there ends up being any ABI breakage or if this turns out to be better for some processes but not others.