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

add better error handling if treetime fails #1023

Merged

Conversation

anna-parker
Copy link
Collaborator

@anna-parker anna-parker commented Aug 11, 2022

Resolves #1013

Description of proposed changes

Hi, after #1013 it appears that errors in augur (refine) that occur while running treetime are not handled in a clear way in augur, and give a very long and not very understandable error message. This is a small change that will make the errors more understandable.

For example the output of the error described in the issue above will change from
image
to
image

Testing

  • install augur and treetime in local environment (branch fix/augur_bug has slightly better error messages but will hopefully soon be merged into the master)
  • create an Exception in treetime, I added a line in _exp_lt to force and error in this function, as this is where the error arose in the issue above, e.g.
        """
        Parameters
        ----------

         t : float
            time to propagate

        Returns
        --------

         exp_lt : numpy.array
            Array of values exp(lambda(i) * t),
            where (i) - alphabet index (the eigenvalue number).
        """
        log_val = self.mu * t * self.eigenvals
        log_val = np.ones(len(log_val))*750 ##cause an exception
        if any(i > 10 for i in log_val):
            raise ValueError("Error in computing exp(Q * t): Q has positive eigenvalues or the branch length t \n"
                    "is too large. This is most likely caused by incorrect input data. If this error persists \n"
                    "please let us know by filing an issue at: https://github.com/neherlab/treetime/issues")

        return np.exp(log_val)

this can be done with other functions or using data which causes an exception.

  • install modified treetime in local environment (pip install .)
  • run augur refine on the CLI using data from the tests folder:
augur refine -t tests/functional/refine/tree.nwk -a tests/functional/refine/aligned.fasta --metadata tests/functional/refine/metadata.tsv --timetree --root 'min_dev' --coalescent 'opt' --output-tree augur.tree.nwk --output-node augur.branch_lengths.json --divergence-units 'mutations'

Checklist

  • Add a message in CHANGES.md summarizing the changes in this PR. Keep headers and formatting consistent with the rest of the file.

Copy link
Member

@corneliusroemer corneliusroemer left a comment

Choose a reason for hiding this comment

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

Very nice, looks like you use Python's error handling how it's supposed to be used. As treetime often errors it's good to handle this!

@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 61.74%. Comparing base (e8392b3) to head (f3634b8).
Report is 1265 commits behind head on master.

Files Patch % Lines
augur/refine.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1023      +/-   ##
==========================================
- Coverage   61.75%   61.74%   -0.01%     
==========================================
  Files          52       52              
  Lines        6275     6279       +4     
  Branches     1581     1581              
==========================================
+ Hits         3875     3877       +2     
- Misses       2129     2131       +2     
  Partials      271      271              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@corneliusroemer corneliusroemer merged commit 7b74c89 into nextstrain:master Aug 12, 2022
victorlin added a commit that referenced this pull request Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
4 participants