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

Improve sparsetb #17

Open
wants to merge 48 commits into
base: master
Choose a base branch
from
Open

Improve sparsetb #17

wants to merge 48 commits into from

Conversation

s-will
Copy link
Collaborator

@s-will s-will commented Feb 5, 2018

Hi @IanWark, hi @HosnaJabbari
I made progress with the new recomputation-based trace back, replacing most of the trace arrows.
We are now a lot faster than pknots and use less space :) as it should be expected after all our theoretical work.
This is at a point, where finding bugs became difficult, so @IanWark could you please test on (smaller) benchmark instances?
(I rewrote this branch and opened a new PR)

s-will added 25 commits January 24, 2018 18:11
in place of three matrices X,mloop10/01/00 only use two X,mloop1/0
 - use stricter candidate criterion (i.e. optimal branch is non-decomposign).

 without recomputation, maintaining less candidates (due to new criterion)
 requires more trace arrows, such that space consumption gets worse
  Failing example:
  ACGAUGCAUGCGACCAGCGGCACGAUCGAGCGCGAUCGAUCGAUCGAGCAACGAGCUAGCGCGACGAUCGUACGGU
 - use M.get notation for all get methods that do no additional computation
 - rename all remaining get_* methods (which perform more computation, e.g.
   initialization, than the Matrix calls get method) to calc_*
…uggy

\time -f "%U %M" ./CCJ  CAUGCGACCAGCGGCACGAUCGAGCGCGAUCGAUCGAUCGAGCAACGAGCUAGCGCGACGAUCGUA
P(2,64): tracing PK(2,24,32,56) e:-977 and PK(25,31,57,64) e:-472
recompute_PX 2 22 33 56 0
Command terminated by signal 11
  - simplify candidate lists
      * use SimpleMap (as originally intended) for single lists
      * use SimpleMap as well as map storing the single lists;
        use extra index for space/time trade-off
  - simplify ta_key_pair(): no need to store the indices k and l
  - add recomputation for e_intP
  - add check for sufficient TURN in calc_PMiloop
  NOTE: this commit turns off garbage collection;
        recomputation of PX and gc is currently not compatible, ie.
        GC removes trace arrows that are necessary for recomputing.
@s-will
Copy link
Collaborator Author

s-will commented Feb 5, 2018

I added my rudimentary unit tests for the matrices classes and class Index4D; currently, one can test with g++ matrices_test.cpp && ./a.out or similar; of course it would be nice to have this better integrated with cmake (and/or a testing framework)

@s-will
Copy link
Collaborator Author

s-will commented Feb 7, 2018

Hi @IanWark, this fixed the wrong energy/trace of
AUGGGGGCCCAAGGUGAGAUGAAGCUGUAGUCUCACUGGAAGGACUAGAGGUUAGAGGAGACCCCCCCAAAACAAAAAACAGCA

@HosnaJabbari, I still think the other differences (all due to< TURN constraints) are actually due to a bugfix ,e.g.
PKB42
UUUAAAUGGGCGAGCGGCACCGCCCGCCAAAACAAACGG
......(([[[[.[[[[))]]]].]]]]........... -8.01
........((((.(((....))).))))........... -7.61
predicts a base pair (16,19) in the master version, which should not be possible.
Please comment.

s-will and others added 16 commits February 7, 2018 12:24
When tracing back to candidate, the target energy must be retrieved
from candidate. To simplify handling, set the correct target energy
already in the generic decomposition method.
 -- use Index4D class and MType
 -- change interface, such that source information can be omitted in tas
 -- store only two indices depending on type
 -- deactivate currently incompatible garbage collection code
  -- two mixed-up indices in POiloop trace back
  -- extend can_pair to rule out j-i<=TURN
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