Skip to content

File and Memory Optimization - #31

Open
noahares wants to merge 4 commits into
BenoitMorel:mainfrom
noahares:feat_file_and_mem_opt
Open

File and Memory Optimization#31
noahares wants to merge 4 commits into
BenoitMorel:mainfrom
noahares:feat_file_and_mem_opt

Conversation

@noahares

Copy link
Copy Markdown
Collaborator

This PR improves 2 things:

  1. Reduced memory usage during reconciliation sampling: No longer keep all reconciliations across all families around to estimate originations, but update them for each family after it is sampled.
  2. Change most output files to TSV: This makes it easier to process files inside alerax and also unifies the output format. Added headers where they were missing and changed "," to "\t" for the files that were csv-like. This will break some post-processing scripts but is the preferable long term solution for output files. That's why I increased the AleRax version string.

This PR involves no functional changes.

@StefanFlaumberg

Copy link
Copy Markdown
Contributor

Hi @noahares,
A very minor request from me: could you not update the version to v1.5.0 for now, but make it v1.4.3?
The rationale is that there is yet little difference from v1.4.0 and that I'd like to add some previously mentioned changes before v1.5.0. The changes are 90% ready, I wanted to add them much earlier, but got distracted by several projects with the IQ-Tree team for longer than I expected. The changes are namely:

  • minor simplification for root search (ready)
  • minor fix for checkpointing (almost ready)
  • fix reading ALE objects (as an alternative to input gene tree distributions) and fix MAD rooting (ready, but I need to check the code with a fresh eye)
  • reimplement your custom-branch-params code for the current version (not ready yet)
  • implement the linear-time-complexity transfer sum algorithm (ready and tested, but a minor update might be needed due to other changes)

I think I could be preparing and sending these changes during the next 2-3 weeks.

If it doesn't align well with your plans, just ignore it. Great changes here btw!

Best,
Stefan

@noahares

noahares commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @StefanFlaumberg,
The idea behind pushing it to 1.5.0 was that it will break backwards compatibility with existing post-processing scripts.
Happy to hear about the progress you made! Looking forward to merge it.
Best,
Noah

@StefanFlaumberg

Copy link
Copy Markdown
Contributor

Hi Noah @noahares,

Great, I hope to be able to send the first part of the changes already this weekend.

Regarding the backwards compatibility with post-processing scripts:
Then maybe it's just the right time to add also the changes for the reconciliation tree format that I proposed before and that were deferred exactly not to create backwards-compatibility issues? The current text-based output has the drawback of not providing the full reconciliation scenario, and those changes fix the drawback (here is the description, and here is the (reversed) code). Please have a look when you have time.

Best,
Stefan

@noahares

noahares commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Yes that would a good time to do that as well. I will be away for the next 2 weeks but I will have a look at it once I am back.

@StefanFlaumberg

Copy link
Copy Markdown
Contributor

Hi @noahares,

I have some comments after looking through your changes, hope you'll find them helpful:

Two similar bugs at lines 312 and 319 in AleOptimizer.cpp. The if-conditions were originally put there to eliminate the leading delimiter, but now they just prevent the D rate from being written. The right code in both cases should be similar to that of the upstream version (no delimiter in the second statement!):

if (...)
  ratesOs << "\t";
ratesOs << ...;

Similar bugs occur as well in the Scenario::saveEventsCounts() and Scenario::saveEventsHeader() functions. There, the right code in both cases should be (no delimiter in the second statement!):

if (i != 0)
  os << "\t";
os << ...;

Also, if possible, the added blank lines 437, 444, 492 in AleOptimizer.cpp are better be removed to comply with the general code style of the project, in which there are no blank lines in function bodies (unfortunately, this cannot be hardcoded in the .clang-format file with reasonable flexibility).

Best regards,
Stefan

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