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

Index bounds2 #897

Merged
merged 14 commits into from
Dec 18, 2023
Merged

Index bounds2 #897

merged 14 commits into from
Dec 18, 2023

Conversation

shawnlaffan
Copy link
Owner

@shawnlaffan shawnlaffan commented Dec 17, 2023

Fixes several of the issues that motivated #895

Still to do:

  • set canonical and pretty flags on the JSON structure. Then it can be added to the repo and releases with minimal changes each time indices are updated. (Done in 37ee273)

unit_interval is a subset of non-negative
so needs to be checked first.
This also means we don't need the hard coded bounds.
use Biodiverse::BaseData in get_calculation_metadata
so it is not needed in calling code that just wants
defaults.

And add caching to get_calculations after profiling
showed it was a bottleneck.
The current list has been there for several
versions and makes startup slower when they
are not needed, e.g. in several tests.
Otherwise all calls have to load it,
and it is not fast.
This avoids loading some heavy modules when not needed.
Should really move the nbr count checks to a separate
method and call it if calculations and nbr_list_count
args are passed.
Profiling shows they take about a second or
more in total on Windows, and this adds up
across >60 tests.
An errant backslash was creating a scalar ref
Now that we require tree branches to be non-negative
we can be sure the index values are also non-negative.
@shawnlaffan shawnlaffan merged commit 47d42f1 into master Dec 18, 2023
8 checks passed
@shawnlaffan shawnlaffan deleted the index_bounds2 branch December 18, 2023 21:33
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.

1 participant