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

1216 benchmark #1261

Merged
merged 17 commits into from
Nov 21, 2024
Merged

1216 benchmark #1261

merged 17 commits into from
Nov 21, 2024

Conversation

pschultzendorff
Copy link
Contributor

Proposed changes

This adds a run_profiler script, which profiles a selected benchmark with viztracer and displays the results. This is a first implementation of such a feature and open to changes in the future. The plan is that people can try it out in the coming weeks and at some point we update based on our findings of what is actually needed in practice. Adresses #1216.

In addition, this adds a new benchmark case with 64 fractures based on this article.

Code written by @Yuriyzabegaev and me.

Types of changes

  • Minor change (e.g., dependency bumps, broken links).
  • Bugfix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Testing (contribution related to testing of existing or new functionality).
  • Documentation (contribution related to adding, improving, or fixing documentation).
  • Maintenance (e.g., improve logic and performance, remove obsolete code).
  • Other:

Checklist

Put an x in the boxes that apply or explain briefly why the box is not relevant.

  • The documentation is up-to-date.
  • Static typing is included in the update.
  • This PR does not duplicate existing functionality.
  • The update is covered by the test suite (including tests added in the PR).
  • If new skipped tests have been introduced in this PR, pytest was run with the --run-skipped flag.

@pschultzendorff pschultzendorff linked an issue Nov 8, 2024 that may be closed by this pull request
@pschultzendorff
Copy link
Contributor Author

@IvarStefansson we called the new runscript run_profiler.py and placed it in src/porepy/examples. Do you think these are appropriate choices or would you suggest something else?

@keileg
Copy link
Contributor

keileg commented Nov 8, 2024

I removed the link to #1216 since that topic is bigger than what is covered in this PR

@keileg
Copy link
Contributor

keileg commented Nov 8, 2024

I will have a more thorough look at this, including running the benchmark, but it will not be before second half of next week.

@keileg keileg self-assigned this Nov 8, 2024
Yury Zabegaev and others added 2 commits November 8, 2024 14:24
The benchmark was specified in terms of Dirichlet conditions on the left
and right boundaries. Updated type and values accordingly.
Copy link
Contributor

@keileg keileg left a comment

Choose a reason for hiding this comment

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

Some comments based on my reading of the code. It is clear that this wlil be an iterative effort, but let's make sure we have a reasonable starting point before letting this loose into the wild.

My attempt at viewing the results in a browser failed, so I'll need to ask for some assistance at some point.

src/porepy/examples/run_profiling.py Outdated Show resolved Hide resolved
src/porepy/examples/run_profiling.py Outdated Show resolved Hide resolved
@Yuriyzabegaev
Copy link
Contributor

Yuriyzabegaev commented Nov 11, 2024

@keileg about viewing the results in the browser, I can't say that this solution is the most robust - sometimes it just fails with an error. What works for me is to reload the page until it works, it usually take 2-5 reloads. The usual time to "open" the data for me is not more than 10 seconds. I checked in Chrome and Brave.

@pschultzendorff
Copy link
Contributor Author

@Yuriyzabegaev and @keileg: I haven't experienced any issues with the results not loading in Firefox so far. Perhaps you can try that and see if it works more reliable for you.

@IvarStefansson
Copy link
Contributor

@IvarStefansson we called the new runscript run_profiler.py and placed it in src/porepy/examples. Do you think these are appropriate choices or would you suggest something else?

Do you mean run_profiling.py? And I don't know about the placement without some more context/discussion.

src/porepy/examples/run_profiling.py Outdated Show resolved Hide resolved
src/porepy/examples/run_profiling.py Outdated Show resolved Hide resolved
@keileg
Copy link
Contributor

keileg commented Nov 21, 2024

Regarding placement of run_profiling.py: Let's make a subfolder of applications, called profiling and put it there. It might be we decide to move it later on, but right now, this seems a fair solution.

Copy link
Contributor

@keileg keileg left a comment

Choose a reason for hiding this comment

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

Thanks for the effort

@Yuriyzabegaev Yuriyzabegaev merged commit 53e88f9 into develop Nov 21, 2024
5 checks passed
@Yuriyzabegaev Yuriyzabegaev deleted the 1216_benchmark branch November 21, 2024 11:48
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