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

auto_flush_edges #2183

Merged
merged 1 commit into from Jun 7, 2023
Merged

auto_flush_edges #2183

merged 1 commit into from Jun 7, 2023

Conversation

GertjanBisschop
Copy link
Member

As discussed in #2181 and #2182.

Note the self->num_buffered_edges = 0; in msp_reset(). When resetting the simulation there might be leftover edges in the buffer if the simulation ended with an error. This was the case for test_demographic_events.

@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

Merging #2183 (9878a78) into main (461dfd2) will increase coverage by 0.04%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2183      +/-   ##
==========================================
+ Coverage   91.41%   91.46%   +0.04%     
==========================================
  Files          20       20              
  Lines       11258    11253       -5     
  Branches     2298     2297       -1     
==========================================
+ Hits        10291    10292       +1     
+ Misses        526      523       -3     
+ Partials      441      438       -3     
Flag Coverage Δ
C 91.46% <71.42%> (+0.04%) ⬆️
python 98.68% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/msprime.c 86.12% <71.42%> (+0.13%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 461dfd2...9878a78. Read the comment docs.

Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Minor tweak suggested

lib/msprime.c Outdated
const double *node_time = self->tables->nodes.time;

tsk_bug_assert(parent < (tsk_id_t) self->tables->nodes.num_rows);
if (self->num_buffered_edges > 0) {
last_edge = self->buffered_edges + self->num_buffered_edges - 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tsk_edge_t *edge, last_edge;

last_edge = self->buffered_edges [self->num_buffered_edges - 1]
if (last_edge.parent != parent) {

is a bit more efficient (and obvious I think - I don't know why I was so fond of pointer addition tricks in the old days!)

@mergify mergify bot merged commit 52abee1 into tskit-dev:main Jun 7, 2023
15 of 16 checks passed
@GertjanBisschop GertjanBisschop mentioned this pull request Jul 21, 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

2 participants