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

PetabJL integration #1089

Merged
merged 27 commits into from
Aug 8, 2023
Merged

PetabJL integration #1089

merged 27 commits into from
Aug 8, 2023

Conversation

PaulJonasJost
Copy link
Collaborator

@PaulJonasJost PaulJonasJost commented Jun 28, 2023

Broaden the usability of the Julia objective with a PEtab_jl objective, that is meant to be used for integration of PEtab.jl

The objective has the option to precompile its module which speeds the multiprocessing optimization up (as otherwise the module which is the time consuming part will always have to be reloaded). Currently if I run the code manually it works fine (even exact same code as the tests) but when running the tests it fails to precompile. So the precompilation is turned off in the tests.

@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2023

Codecov Report

Merging #1089 (b2afd81) into develop (160c2a8) will decrease coverage by 3.69%.
Report is 364 commits behind head on develop.
The diff coverage is 88.08%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff             @@
##           develop    #1089      +/-   ##
===========================================
- Coverage    88.16%   84.48%   -3.69%     
===========================================
  Files           79      148      +69     
  Lines         5257    11593    +6336     
===========================================
+ Hits          4635     9794    +5159     
- Misses         622     1799    +1177     
Files Changed Coverage Δ
pypesto/engine/mpi_pool.py 0.00% <0.00%> (ø)
pypesto/objective/amici/__init__.py 100.00% <ø> (ø)
pypesto/objective/amici/amici.py 85.15% <ø> (ø)
pypesto/objective/amici/amici_calculator.py 86.36% <ø> (ø)
pypesto/objective/amici/amici_util.py 81.53% <ø> (ø)
pypesto/objective/base.py 87.62% <ø> (-0.10%) ⬇️
pypesto/objective/finite_difference.py 94.16% <ø> (ø)
pypesto/objective/function.py 94.91% <ø> (+19.91%) ⬆️
pypesto/objective/jax/__init__.py 100.00% <ø> (ø)
pypesto/objective/jax/base.py 91.56% <ø> (ø)
... and 138 more

@PaulJonasJost PaulJonasJost self-assigned this Jun 28, 2023
@PaulJonasJost PaulJonasJost marked this pull request as ready for review June 28, 2023 15:01
Copy link

@sebapersson sebapersson left a comment

Choose a reason for hiding this comment

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

Thank you so much for working on this PEtab.jl importer :)

I have added a few comments to allow more efficient usage of the Julia PEtab importer.

pypesto/objective/julia/petabJl.py Show resolved Hide resolved
pypesto/objective/julia/petab_jl_importer.py Show resolved Hide resolved
pypesto/objective/julia/petab_jl_importer.py Show resolved Hide resolved
Copy link
Member

@yannikschaelte yannikschaelte left a comment

Choose a reason for hiding this comment

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

would be good to be able to test precompilation too

hess = self.petab_jl_problem.computeHessian
x_names = np.asarray(self.petab_jl_problem.θ_estNames)

# call the super super constructor
Copy link
Member

Choose a reason for hiding this comment

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

super super super

pypesto/objective/julia/petabJl.py Outdated Show resolved Hide resolved
pypesto/objective/julia/petab_jl_importer.py Show resolved Hide resolved
test/julia/test_pyjulia.py Outdated Show resolved Hide resolved
test/julia/test_pyjulia.py Outdated Show resolved Hide resolved
@PaulJonasJost PaulJonasJost merged commit 2eb0d15 into develop Aug 8, 2023
16 checks passed
@PaulJonasJost PaulJonasJost deleted the petab_jl_importer branch August 8, 2023 08:07
@PaulJonasJost PaulJonasJost mentioned this pull request Oct 2, 2023
PaulJonasJost added a commit that referenced this pull request Oct 2, 2023
* Use cloudpickle for pickling dynesty sampler (#1094)

Cloudpickle handles ABC-derived classes properly, whereas the latest dill version on pypi (0.3.6) does not.

Cloudpickle is already installed with pypesto, so there seems to be no need to get dill on top.

* Restrict fval magnitude in waterfall with order_by_id (#1090)

* restrict waterfall with order_by_id to fvals < 1e100
* add constant for parameters/waterfall plot maximum value

* Add startpoint_method to Problem (#1093)

It's not intuitive that `x_guesses` is part of `Problem`, but `startpoint_method`s are handled separately. See also discussion in #1017.

This patch
* adds `startpoint_method` to `Problem`
* during PEtab import, sets `Problem.startpoint_method` based on the PEtab problem
* deprecates `startpoint_method` arguments where also `Problem` is passed

Closes #1035

* Small initialize fix (#1095)

Censoring bounds should not be reinitialized when calculators are reinitialized.

* Added Falco et al to bib. (#1100)

* fix storage (#1099)

* Hierarchical parameter plot fix (#1106)

* Fix parameter plot

If, in any case (because some inner optimization failed, simulation failed, or some other reason) and there is a`optimize_result`dictionary in `result.optmize_result.list` which doesn't contain the `INNER_PARAMETERS` key and its item, the parameter plot would fail at line 381.

This is a fix to make it robust to this. If any of the `optimize_result` objects has a `INNER_PARAMETER` key, then the other ones will be filled with NaN values, and plotted.

* Dilan review change

Co-authored-by: Dilan Pathirana <[email protected]>

* Update pypesto/visualize/parameters.py

Co-authored-by: Daniel Weindl <[email protected]>

* Add comments

* Add comment

Testing the quality check

* Fix quality check error

---------

Co-authored-by: Dilan Pathirana <[email protected]>
Co-authored-by: Daniel Weindl <[email protected]>

* Fix startpoint sampling for hierarchical optimization (#1105)

* Fix startpoint sampling for hierarchical optimization

See #1096

* fixed pars

* fixup

---------

Co-authored-by: Doresic <[email protected]>

* PetabJL integration (#1089)

* SingleCore engine works, multicore has problems with pickling even though manual pickling works

* intermediate commit

* Working tests.

* small changes needs verification

* created base test to check for integration into python.

* Added julia module file for test

* Added test for objective function evaluation

* Added values for evaluation

* Added installation of packages to Workflow.

* Readded the Julia files that for some reason were deleted.

* Removed PetabJL from default pypesto.petab import

* Temporariy added amici as dependency. Needs to be taken care of appropriately.

* Moved Importer to objective/julia

* Deactivated precompilation in test

* Integrated suggestions

* add comments

* add sundials to ci

---------

Co-authored-by: Yannik Schälte <[email protected]>

* Fix #1108 (#1109)

* enable setting the y limits of a waterfall plot by using the y_limits argument
* use default y limits for zoomed in starts
* fix #1108

Co-authored-by: Paul Jonas Jost <[email protected]>

* SacessOptimizer: retry reading, delay deleting (#1110)


* delete temporary files only after *all* were read successfully
* retry reading temp file upon error to work around potential filesystem latency issues

* SacessOptimizer: Fix logging with multiprocessing (#1112)

* SacessOptimizer: Fix logging with multiprocessing

* Fix: `ValueError: setting an array element with a sequence.`

* Other platform tests (#1113)

* blind attempt mac

* brew installs

* try python 3.11 numba

* try windows

* fix cache

* stuff

* stuff

* stuff

* stuff

* minimal test

* add basic workflow test

* Update .github/workflows/ci.yml

Co-authored-by: Daniel Weindl <[email protected]>

* close figures

---------

Co-authored-by: Daniel Weindl <[email protected]>

* SacessOptimizer: tmpdir option (#1115)

SacessOptimizer:
* Add option to pass a custom directory for temporary files
* Make the default temporary file paths unique, to avoid name collisions when running multiple SacessOptimizer instances from within the same working directory

* Notebook on differences (#1098)

* Notebook on differences

* simplify some parts; add some more details and visualizations

* minor edit

* add workflow comparison to docs

* integrated suggestions

* integrated suggestions 2

* fixup

* removed personal info.
changed nlop objective appropriatly

---------

Co-authored-by: Yannik Schälte <[email protected]>
Co-authored-by: Yannik Schaelte <[email protected]>

* Documentation fixes (#1120)

* document hierarchical optimization sources; add aesara+jax to objective docs

* document origin of the mh + pt samplers

* fix typo

* fix typo

* Update pypesto/sample/parallel_tempering.py

Typo

* streamline doc deps

* update setup req versions

* try cyipopt fix

* test

* tesT

* test

* fix test_visualize::test_optimization_scatter_with_x_None

* test

* fixup

---------

Co-authored-by: Paul Jonas Jost <[email protected]>

* update changelog+version

* Changed Codeowners and contact to accomodate changes in personell (#1123)

* Updated version and Changelog (#1122)

* Updated version and Changelog

* Updated Changelog

* Update CHANGELOG.rst

---------

Co-authored-by: Daniel Weindl <[email protected]>
Co-authored-by: Maren Philipps <[email protected]>
Co-authored-by: Doresic <[email protected]>
Co-authored-by: Polina Lakrisenko <[email protected]>
Co-authored-by: Dilan Pathirana <[email protected]>
Co-authored-by: Yannik Schälte <[email protected]>
Co-authored-by: Yannik Schaelte <[email protected]>
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.

5 participants