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

hdf5 reader for ensemble prediction #681

Merged
merged 13 commits into from
Jul 1, 2021

Conversation

MerktSimon
Copy link
Contributor

@MerktSimon MerktSimon commented Jun 3, 2021

Writing already exists:

def write_ensemble_prediction_to_h5(ensemble_prediction: EnsemblePrediction,

Useful e.g. for large sample based predictions, which take several hours and a lot of memory, if one wants to come back to earlier predictions.

Progress

  • add basic functional code
  • add upper/lower bound support
  • implement test
  • polish code

@MerktSimon MerktSimon marked this pull request as draft June 3, 2021 00:12
@MerktSimon MerktSimon changed the title Implement function to read ensemble prediction from hdf5 file hdf5 reader for ensemble prediction Jun 3, 2021
Copy link
Collaborator

@PaulJonasJost PaulJonasJost left a comment

Choose a reason for hiding this comment

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

Will work on it, thanks for setting up something already 👍

base = Path(base_path)

# open file
f = h5py.File(input_file, 'r')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather make it into With h5py.File(input_file, 'r') as f: that way we ensure it is closed after reading.

prediction_id = str(f[os.path.join(base, PREDICTION_ID)][()], 'utf-8')

pred_res_list = []
for i_pred in range(5): #len(f.keys())-1):
Copy link
Collaborator

Choose a reason for hiding this comment

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

need to replace range(5). Probably easier to just loop over the keys of f and skip prediction_id?

@PaulJonasJost
Copy link
Collaborator

Ok, I put the reader function into util, as there were import problem otherwise. Will write correct reader and writer function/classes at some later timepoints.
Will also rework the saving function at a later date, for now it only works without base_path.

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2021

Codecov Report

Merging #681 (5cdc17a) into develop (a6aef21) will increase coverage by 43.87%.
The diff coverage is 77.41%.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop     #681       +/-   ##
============================================
+ Coverage    46.50%   90.37%   +43.87%     
============================================
  Files           96       96               
  Lines         6400     6463       +63     
============================================
+ Hits          2976     5841     +2865     
+ Misses        3424      622     -2802     
Impacted Files Coverage Δ
pypesto/ensemble/__init__.py 100.00% <ø> (ø)
pypesto/predict/result.py 88.03% <67.85%> (-6.42%) ⬇️
pypesto/ensemble/ensemble.py 82.44% <75.00%> (+21.13%) ⬆️
pypesto/ensemble/utils.py 71.71% <81.66%> (+14.14%) ⬆️
pypesto/store/__init__.py 100.00% <100.00%> (ø)
pypesto/objective/amici_calculator.py 93.33% <0.00%> (+1.66%) ⬆️
pypesto/objective/base.py 88.76% <0.00%> (+1.68%) ⬆️
pypesto/optimize/optimizer.py 90.09% <0.00%> (+3.45%) ⬆️
pypesto/optimize/optimize.py 94.00% <0.00%> (+6.00%) ⬆️
pypesto/ensemble/constants.py 91.83% <0.00%> (+6.12%) ⬆️
... and 60 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6aef21...5cdc17a. Read the comment docs.

prediction_id=AMICI_X,
engine=engine)

fn = 'test_file.hdf5'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe better with https://docs.python.org/3/library/tempfile.html
What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

do not remember why I have started avoiding it tbh :D

@PaulJonasJost PaulJonasJost marked this pull request as ready for review June 24, 2021 10:07
Copy link
Member

@dilpath dilpath left a comment

Choose a reason for hiding this comment

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

👍

pypesto/ensemble/ensemble.py Outdated Show resolved Hide resolved
pypesto/ensemble/ensemble.py Outdated Show resolved Hide resolved
pypesto/ensemble/ensemble.py Outdated Show resolved Hide resolved
pypesto/ensemble/utils.py Show resolved Hide resolved
pypesto/ensemble/utils.py Outdated Show resolved Hide resolved
pypesto/ensemble/utils.py Outdated Show resolved Hide resolved
pypesto/ensemble/utils.py Outdated Show resolved Hide resolved
pypesto/ensemble/utils.py Outdated Show resolved Hide resolved
pypesto/ensemble/utils.py Show resolved Hide resolved
pypesto/predict/result.py Outdated Show resolved Hide resolved
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
pypesto/ensemble/utils.py Outdated Show resolved Hide resolved
pypesto/ensemble/utils.py Outdated Show resolved Hide resolved
@PaulJonasJost PaulJonasJost merged commit f37f050 into develop Jul 1, 2021
@PaulJonasJost PaulJonasJost deleted the read_ensemble_prediction_hdf5 branch July 1, 2021 07:42
@yannikschaelte yannikschaelte mentioned this pull request Aug 2, 2021
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.

4 participants