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

Adapt the free end gap configuration of the alignment #96

Closed
2 of 13 tasks
rrahn opened this issue May 26, 2020 · 2 comments
Closed
2 of 13 tasks

Adapt the free end gap configuration of the alignment #96

rrahn opened this issue May 26, 2020 · 2 comments
Assignees
Labels
ready to tackle This story was discussed and can be immidietly tackled
Milestone

Comments

@rrahn
Copy link
Contributor

rrahn commented May 26, 2020

Description

Following the discussion on #131 adapt the free end configuration.

seqan3::align_cfg::method_global:    [required]

construction

seqan3::align_cfg::method_global global
{
seqan3::align_cfg::free_end_gaps_sequence1_leading{true},
seqan3::align_cfg::free_end_gaps_sequence2_leading{true},
seqan3::align_cfg::free_end_gaps_sequence1_trailing{true},
seqan3::align_cfg::free_end_gaps_sequence2_trailing{true}
}; // verbose has constructor with 4 constructor elements

later access and set

global.free_end_gaps_sequence1_leading = true/ false;
global.free_end_gaps_sequence2_leading = true/ false;
global.free_end_gaps_sequence1_trailing = true/ false;
global.free_end_gaps_sequence2_trailing = true/ false;

or by seqan::get

auto && global = seqan3::get<seqan3::align_cfg::method_global>(config);

global.free_end_gaps_sequence1_leading = true/ false;
global.free_end_gaps_sequence2_leading = true/ false;
global.free_end_gaps_sequence1_trailing = true/ false;
global.free_end_gaps_sequence2_trailing = true/ false;

query if configuration was piped

Acceptance Criteria

  • gap configuration can be set over method_global liek described above

Tasks

  • Introduce strong types free_end_gaps_sequence[1/2]_[leading/trailing]
  • Make seqan3::method_global a pipeable ocnfig element (not a tag), that is constructible from the 4 strong_types
  • Make alignment algorithm use the method_global end gap configuration instead of using the aligned_ends configuration
  • Remove the seqan3::align_cfg::aligned_ends configuration

Definition of Done

  • Implementation and design approved
  • Unit tests pass
  • Test coverage = 100%
  • Microbenchmarks added and/or affected microbenchmarks < 5% performance drop
  • API documentation added
  • Tutorial/teaching material added
  • Test suite compiles in less than 30 seconds (on travis)
  • Changelog entry added
@rrahn rrahn added the needs refinement A story that was not discussed and/or estimated by the team yet but is planned for upcoming sprints. label May 26, 2020
@rrahn rrahn added this to the Sprint 5 milestone May 26, 2020
@rrahn rrahn modified the milestones: Sprint 5, Sprint 6 Jun 8, 2020
@smehringer smehringer self-assigned this Jun 26, 2020
@smehringer
Copy link
Member

smehringer commented Jul 3, 2020

We decided to introduce shortcuts:

// gap short cuts
struct free_end_gaps_sequence1;
struct free_end_gaps_sequence2;
struct free_end_gaps_all;

// ... extra constructor in method_global that sets the four members for each short cut
    constexpr method_global(seqan3::align_cfg::free_end_gaps_sequence1) noexcept :
        free_end_gaps_sequence1_leading{seqan3::align_cfg::free_end_gaps_sequence1_leading{true}},
        free_end_gaps_sequence2_leading{seqan3::align_cfg::free_end_gaps_sequence2_leading{false}},
        free_end_gaps_sequence1_trailing{seqan3::align_cfg::free_end_gaps_sequence1_trailing{true}},
        free_end_gaps_sequence2_trailing{seqan3::align_cfg::free_end_gaps_sequence2_trailing{false}}
    {}
     constexpr method_global(seqan3::align_cfg::free_end_gaps_sequence2) noexcept {}// ...
     constexpr method_global(seqan3::align_cfg::free_end_gaps_all) noexcept {}// ...

// usage:
auto cfg = seqan3::align_cfg::method_global{seqan3::align_cfg::free_end_gaps_sequence1};

Why not directly introducing method_global short cuts:

// gap short cuts
inline constexpr method_global_free_end_gaps_sequence1 = method_global
{
    free_end_gaps_sequence1_leading{true},
    free_end_gaps_sequence2_leading{false},
    free_end_gaps_sequence1_trailing{true},
    free_end_gaps_sequence2_trailing{false}
};

inline constexpr method_global_free_end_gaps_sequence2 = method_global{...};
inline constexpr method_global_free_end_gaps_all = method_global{...};

// usage
auto cfg = seqan3::align_cfg::method_global_free_end_gaps_sequence1;


auto cfg = seqan3::align_cfg::method_global_overlap;

Names may have to be discussed

@marehr
Copy link
Member

marehr commented Jul 3, 2020

Short note, we don't necessarily need an extra method_global_ prefix name, because

inline constexpr method_global free_end_gaps_sequence1
{
    free_end_gaps_sequence1_leading{true},
    free_end_gaps_sequence2_leading{false},
    free_end_gaps_sequence1_trailing{true},
    free_end_gaps_sequence2_trailing{false}
};

seqan3::align_cfg::method_global{free_end_gaps_sequence1}; // is still valid

but I see you point that

auto cfg = free_end_gaps_sequence1 | ...

Could be piped directly which would be bad. I'm a bit against all those method_global_ prefixes.

We could also do a construct like this:

struct free_end_gaps
{
seqan3::align_cfg::free_end_gaps_sequence1_leading free_end_gaps_sequence1_leading{};
seqan3::align_cfg::free_end_gaps_sequence2_leading free_end_gaps_sequence2_leading{};
seqan3::align_cfg::free_end_gaps_sequence1_trailing free_end_gaps_sequence1_trailing{};
seqan3::align_cfg::free_end_gaps_sequence2_trailing free_end_gaps_sequence2_trailing{};
};

inline constexpr free_end_gaps free_end_gaps_sequence1
{
    free_end_gaps_sequence1_leading{true},
    free_end_gaps_sequence2_leading{false},
    free_end_gaps_sequence1_trailing{true},
    free_end_gaps_sequence2_trailing{false}
};

struct method_global : free_end_gaps
{
//..
};

// you need to use method_global as data-holder which is different from method_global

method_global{free_end_gaps{...}};
method_global{{...}};
method_global{...};

seqan3::align_cfg::method_global{seqan3::align_cfg::free_end_gaps{seqan3::align_cfg::free_end_gaps....}}

@rrahn rrahn added ready to tackle This story was discussed and can be immidietly tackled and removed needs refinement A story that was not discussed and/or estimated by the team yet but is planned for upcoming sprints. labels Jul 13, 2020
@rrahn rrahn closed this as completed Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to tackle This story was discussed and can be immidietly tackled
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants