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 MEModel #136

Merged
merged 10 commits into from
May 30, 2024
Merged

add MEModel #136

merged 10 commits into from
May 30, 2024

Conversation

AurelienJaquier
Copy link
Collaborator

@AurelienJaquier AurelienJaquier commented May 8, 2024

also small refactoring for EModel, search_pdfs and validate

Also take into account all possible legacy checkpoint paths, that depend on EModelMetadata.as_string(), that has been updated several times

Change-Id: I0ac683e488d7269ec67318aa64d9d57ca0a06ed0
@AurelienJaquier AurelienJaquier self-assigned this May 8, 2024
@codecov-commenter
Copy link

codecov-commenter commented May 8, 2024

Codecov Report

Attention: Patch coverage is 62.50000% with 57 lines in your changes are missing coverage. Please review.

Project coverage is 60.23%. Comparing base (8d5b0ab) to head (c780aa1).

Files Patch % Lines
bluepyemodel/emodel_pipeline/memodel.py 0.00% 20 Missing ⚠️
bluepyemodel/tools/search_pdfs.py 15.38% 11 Missing ⚠️
bluepyemodel/emodel_pipeline/plotting.py 0.00% 10 Missing ⚠️
bluepyemodel/emodel_pipeline/emodel.py 85.36% 6 Missing ⚠️
bluepyemodel/tools/utils.py 81.81% 4 Missing ⚠️
bluepyemodel/access_point/access_point.py 0.00% 3 Missing ⚠️
bluepyemodel/optimisation/optimisation.py 0.00% 2 Missing ⚠️
bluepyemodel/validation/validation.py 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #136      +/-   ##
==========================================
+ Coverage   60.07%   60.23%   +0.15%     
==========================================
  Files         109      110       +1     
  Lines        7838     7841       +3     
==========================================
+ Hits         4709     4723      +14     
+ Misses       3129     3118      -11     

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

Jaquier Aurélien Tristan added 4 commits May 8, 2024 10:33
Change-Id: Ied1a0d85e778c2b937e4ef49233cae546da350ba
Change-Id: I0048cc88c3295519dbb39f27fde512eafbe6862d
Change-Id: I4ff43ce5d81a8a0a6b0fee622b413929475a2392
… memodel

Conflicts:
	bluepyemodel/validation/validation.py

Change-Id: I2a6555a53a83c9bde8adaa0a67c782312ce76112
@AurelienJaquier AurelienJaquier marked this pull request as ready for review May 28, 2024 11:58
Jaquier Aurélien Tristan added 4 commits May 28, 2024 14:03
Change-Id: I5e782dedc4c34d1e3c30626b8134f4512a92b51d
Change-Id: I55d6a2388fb3fce1e08f78b1149b2b0dc4c6f912
Change-Id: I1c0ca3a4a4396413418cab5e6efa4e7dac2c3e20
Change-Id: I2d419eb0c423ae90888c7c6cd28cc8df09a30559
Copy link
Member

@adrien-berchet adrien-berchet left a comment

Choose a reason for hiding this comment

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

Just minor comments


def get_checkpoint_path(metadata, seed=None):
"""Get checkpoint path. Use legacy format if any is found, if not use latest format."""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Get checkpoint path. Use legacy format if any is found, if not use latest format."""
"""Get checkpoint path. Use legacy format if any is found, else use latest format."""

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed in latest commit


return checkpoint_metadata
return int(seed)
Copy link
Member

Choose a reason for hiding this comment

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

As far as I understand, if search_str is not found in any filename then seed will be None and then int(None) will raise an error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! I changed default seed to be 0 if it is not found in the filename

metadata = EModelMetadata(emodel="L5PC", ttype="t type", iteration_tag="test")
def checkpoint_check(dir, fname, metadata, inner_dir):
f = dir / fname
f.write_text("")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
f.write_text("")
f.touch()

If I'm correct f is a pathlib.Path object, so you can use Path.touch() to create an empty file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, changed in latest commit

Change-Id: Ic44a24779e836f1fd96862b5b7a9cd8604e5e59f
@AurelienJaquier AurelienJaquier merged commit a4db6ac into main May 30, 2024
7 checks passed
@AurelienJaquier AurelienJaquier deleted the memodel branch May 30, 2024 08:23
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.

3 participants