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

No flattening of timepoint specific overrides in jax #2641

Merged
merged 26 commits into from
Feb 5, 2025

Conversation

FFroehlich
Copy link
Member

No description provided.

@FFroehlich FFroehlich changed the base branch from master to develop January 31, 2025 12:39
Copy link

codecov bot commented Jan 31, 2025

Codecov Report

Attention: Patch coverage is 95.70552% with 7 lines in your changes missing coverage. Please review.

Project coverage is 77.16%. Comparing base (c9f698d) to head (e2681e8).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
python/sdist/amici/pysb_import.py 85.18% 4 Missing ⚠️
python/sdist/amici/petab/pysb_import.py 91.30% 2 Missing ⚠️
python/sdist/amici/jax/model.py 87.50% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2641      +/-   ##
===========================================
+ Coverage    77.14%   77.16%   +0.01%     
===========================================
  Files          332      332              
  Lines        23021    23137     +116     
  Branches      1478     1478              
===========================================
+ Hits         17760    17854      +94     
- Misses        5250     5272      +22     
  Partials        11       11              
Flag Coverage Δ
cpp 74.10% <71.42%> (-0.16%) ⬇️
cpp_python 33.39% <28.57%> (-0.03%) ⬇️
petab 39.40% <96.10%> (+0.39%) ⬆️
python 72.21% <79.14%> (-0.03%) ⬇️

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

Files with missing lines Coverage Δ
python/sdist/amici/constants.py 100.00% <100.00%> (ø)
python/sdist/amici/de_model.py 90.30% <100.00%> (+0.14%) ⬆️
python/sdist/amici/de_model_components.py 89.61% <100.00%> (+0.42%) ⬆️
python/sdist/amici/jax/ode_export.py 96.87% <100.00%> (ø)
python/sdist/amici/jax/petab.py 98.03% <100.00%> (+0.61%) ⬆️
python/sdist/amici/petab/sbml_import.py 76.61% <100.00%> (+0.59%) ⬆️
python/sdist/amici/sbml_import.py 79.42% <100.00%> (+0.16%) ⬆️
python/sdist/amici/jax/model.py 87.74% <87.50%> (ø)
python/sdist/amici/petab/pysb_import.py 37.90% <91.30%> (+7.62%) ⬆️
python/sdist/amici/pysb_import.py 82.41% <85.18%> (+0.15%) ⬆️

... and 3 files with indirect coverage changes

@FFroehlich FFroehlich marked this pull request as ready for review February 4, 2025 10:04
@FFroehlich FFroehlich requested a review from a team as a code owner February 4, 2025 10:04
Copy link
Member

@dweindl dweindl left a comment

Choose a reason for hiding this comment

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

Thanks. I'd didn't check super thoroughly, but looks good to me.

pytest.ini Outdated Show resolved Hide resolved
python/sdist/amici/de_model_components.py Show resolved Hide resolved
python/sdist/amici/de_model.py Outdated Show resolved Hide resolved
python/sdist/amici/jax/petab.py Outdated Show resolved Hide resolved
python/sdist/amici/jax/petab.py Outdated Show resolved Hide resolved
pysb_model, name, sigmas
# note that this is a bit iffy since we are potentially using the same symbolic identifier in expressions (w)
# and observables (y). This is not a problem as there currently are no model functions that use both. If this
# changes, I would expect symbol redefinition warnings in CPP models and overwriting in JAX models, but as both
Copy link
Member

Choose a reason for hiding this comment

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

I think these warnings are easily overlooked. Maybe we should turn them into errors. But this will break the import of at least one benchmark collection model that has something named NULL. Fine for now.

python/sdist/amici/pysb_import.py Outdated Show resolved Hide resolved
@FFroehlich FFroehlich enabled auto-merge February 5, 2025 19:33
Copy link

sonarqubecloud bot commented Feb 5, 2025

@FFroehlich FFroehlich added this pull request to the merge queue Feb 5, 2025
Merged via the queue into develop with commit b7bdf63 Feb 5, 2025
38 checks 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