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

Update documentation for web page #3411

Merged
merged 10 commits into from Apr 30, 2024
Merged

Conversation

cmcantalupo
Copy link
Contributor

  • Relates to Documentation updates #3385
  • Read through all existing documentation and re-word/re-flow as necessary to match the new directory structure of the source repository.
  • Documentation about things like "you MUST use the performance cpufreq governor" can be removed/relaxed. We're not just concerned with HPC anymore. Similarly, CPU affinity requirements.
  • HPC specific doc would be nice that discusses CPU governors, rank-to-CPU affinity, OpenMP, MPI, etc.

docs/source/requires.rst Outdated Show resolved Hide resolved
docs/source/runtime.rst Outdated Show resolved Hide resolved
Comment on lines 101 to 103
system administrator. This feature is mandatory if the GEOPM Access Service is
not active on the system. Alternatively, the access can also be managed by the
system administrator using the GEOPM Access Service, if active.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the above paragraph could be reworked a bit. It should discuss the fact that allowlists in msr-safe are required in all usages now. It should also mention the allowlist generation capability of geopmaccess -s and why that's important.

Happy to take a crack at that later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the logging feature in geopmaccess for the minimal list.

It's crucial to note that other Linux mechanisms for CPU power management can
interfere with optimization objectives of GEOPM agents. When deploying an agent
that controls CPU frequency or power limits, it's recommended that the generic
scaling govermernor ``userspace`` is selected.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's important to still mention intel_pstate here. It may interfere with your results depending on what you're trying to do. We should maybe mention userspace alongside performance.

Copy link
Contributor

Choose a reason for hiding this comment

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

performance should be mentioned for when userspace is not available.

consider moving HPC tuning tips to a separate page.

different mechanisms for affinitizing OpenMP threads within CPUs available
to each MPI process. The GEOPM control thread can also be launched as an
application thread or process that can either be part of the primary MPI
application or a completely different MPI application. Due to these factors,
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these features still exist in the Controller and geopmlaunch...
why remove this now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Some of these details should be moved to the non-existent HPC tuning guide and/or references to geopmlaunch should be added.

docs/source/overview.rst Outdated Show resolved Hide resolved
docs/source/runtime.rst Outdated Show resolved Hide resolved
cmcantalupo and others added 10 commits April 30, 2024 14:23
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
…d guide

Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
Co-authored-by: Brad Geltz <brgeltz@gmail.com>
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
…on and geopmaccess

Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
@cmcantalupo cmcantalupo enabled auto-merge (rebase) April 30, 2024 21:24
@cmcantalupo cmcantalupo merged commit 6b51823 into geopm:dev Apr 30, 2024
11 checks passed
bgeltz added a commit to bgeltz/geopm that referenced this pull request May 1, 2024
- Fails with IndexError otherwise.
- Introduced in geopm#3411.

Signed-off-by: Brad Geltz <brad.geltz@intel.com>
@bgeltz bgeltz mentioned this pull request May 1, 2024
cmcantalupo pushed a commit that referenced this pull request May 1, 2024
- Fails with IndexError otherwise.
- Introduced in #3411.

Signed-off-by: Brad Geltz <brad.geltz@intel.com>
@cmcantalupo cmcantalupo added this to the YR24 WW17 milestone May 8, 2024
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.

None yet

3 participants