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 some more logging #5321

Merged
merged 1 commit into from
May 2, 2023
Merged

Conversation

frode-aarstad
Copy link
Contributor

@frode-aarstad frode-aarstad commented May 2, 2023

Issue
Resolves #5276

Approach
Short description of the approach

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Updated documentation
  • Ensured new behaviour is tested

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

@frode-aarstad frode-aarstad self-assigned this May 2, 2023
@frode-aarstad frode-aarstad added release-notes:skip If there should be no mention of this in release notes release-notes:improvement Automatically categorise as improvement in release notes and removed release-notes:skip If there should be no mention of this in release notes labels May 2, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 2, 2023

Codecov Report

Merging #5321 (320a0f1) into main (1e4b7d6) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5321      +/-   ##
==========================================
+ Coverage   73.72%   73.77%   +0.04%     
==========================================
  Files         392      392              
  Lines       26787    26809      +22     
  Branches     2088     2086       -2     
==========================================
+ Hits        19750    19778      +28     
+ Misses       6362     6356       -6     
  Partials      675      675              
Impacted Files Coverage Δ
src/ert/_c_wrappers/enkf/config/field_config.py 97.05% <100.00%> (+0.33%) ⬆️
src/ert/_c_wrappers/enkf/config/surface_config.py 96.87% <100.00%> (+0.87%) ⬆️
src/ert/_c_wrappers/enkf/enkf_main.py 98.25% <100.00%> (+0.05%) ⬆️
src/ert/analysis/_es_update.py 87.00% <100.00%> (+0.92%) ⬆️

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -66,12 +71,15 @@ def load(self, run_path: Path, real_nr: int, ensemble: EnsembleAccessor):
trans = self.input_transformation
data_transformed = field_transform(data, trans) if trans else data
ensemble.save_field(key, real_nr, data_transformed)
_logger.debug(f"load() time_used {time.perf_counter() - t}s")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we reduce the resolution a bit here?
Perhaps something like:

_logger.debug(f"load() time_used {(time.perf_counter() - t):.4f}s")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeas - any more digits does not make any sense

@frode-aarstad frode-aarstad merged commit 1852921 into equinor:main May 2, 2023
37 checks passed
@frode-aarstad frode-aarstad deleted the add-logging branch May 2, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:improvement Automatically categorise as improvement in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add logging to callbacks and data loading
3 participants