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

Integrate the multihit model into the DNSM framework #71

Merged
merged 8 commits into from
Oct 23, 2024

Conversation

willdumm
Copy link
Contributor

This PR replaces #60, and is a companion to https://github.com/matsengrp/dnsm-experiments-1/pull/29

Here we add the multihit model to the DNSM. We chose to do so by storing the multihit model, as an object, as part of the DNSMDataset object, and bringing it into molevol.py.

This is a bit of a break from the previous work, which was able to stay in tensor land rather than object land. E.g. molevol.py now has objects from models.py.

Here is the rationale.

Previously, we could pre-compute the per-site rates and then all future calculations would be relative to those rates. To emphasize, the per-site rates don't depend on the branch lengths or anything.

Here, we have the multihit multipliers being applied after the codon level calculations. These codon calculations depend on the branch lengths and rates in a nonlinear way. Thus we can't precompute rates and then apply them in a straightforward way.

In theory we could stay more in tensor land by precomputing a 64*64 (this is codon by codon, because the hit classes depend on the source and dest codon) offset per site per sequence, and multiplying that after the codon aggregation step. However, that seems wasteful of memory and overkill.

So we have decided to apply the multihit correction using the forward function of the corresponding object, which is exactly what it's meant to do.

In any case, all of this work happens only in the branch length optimization phase of making the DNSMs, and not the transformer backprop.

matsen and others added 3 commits October 22, 2024 16:24
Add multihit model in a few more places

make format

fix shape issue

fix rebase issue, but tests still fail

force_spawn in tests

reformat

switch to serial branch length optimization

multihit works with threading

WIP-- nothing works now

comment

test+fake multihit working

re-enable multihit

perhaps working, fixed device mismatch and switched to nonlog correction application

I think this might have worked

verified working here

cleanup for PR
@willdumm willdumm changed the title 54 multihit dnsm clean Integrate the multihit model into the DNSM framework Oct 23, 2024
Copy link
Contributor

@matsen matsen left a comment

Choose a reason for hiding this comment

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

Some small things here to fix.

netam/hit_class.py Outdated Show resolved Hide resolved
netam/models.py Outdated Show resolved Hide resolved
netam/molevol.py Outdated Show resolved Hide resolved
netam/molevol.py Outdated Show resolved Hide resolved
tests/test_dasm.py Outdated Show resolved Hide resolved
tests/test_dnsm.py Outdated Show resolved Hide resolved
Copy link
Contributor

@matsen matsen left a comment

Choose a reason for hiding this comment

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

Two last nitpicks.

netam/dnsm.py Show resolved Hide resolved
netam/molevol.py Outdated Show resolved Hide resolved
Copy link
Contributor

@matsen matsen left a comment

Choose a reason for hiding this comment

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

Great!

@matsen matsen merged commit a5ef50d into main Oct 23, 2024
1 check passed
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