Axom Dev Guide Updates #1430

rhornung67 · 2024-09-27T22:19:15Z

Summary

This PR clarifies some of the Axom Dev Guide discussion and add some details related to TPLs and updates of those.
It also update some of the release process discussion.

The RTD content for this PR is here: https://axom.readthedocs.io/en/task-rhornung67-devguide-tpls/docs/sphinx/dev_guide/index.html

bmhan12 · 2024-10-02T15:28:48Z

src/docs/sphinx/dev_guide/release.rst

+   <https://github.com/LLNL/axom/issues/new>`_ and note it in the ``known bugs``
+   section of the release notes. Alternatively, if time permits, fix the 
+   bug in a different branch and create a pull request as you would do during
+   regular development. After the bus is resolved and that pull request is


Suggested change

regular development. After the bus is resolved and that pull request is

regular development. After the bug is resolved and that pull request is

Thanks. Fixed.

agcapps · 2024-10-02T21:49:36Z

src/docs/sphinx/dev_guide/updating_tpls.rst

+and configurations of software packages. It has recipes for building each package
+with available variants to customize it to your needs. For example, Axom
+has variants for Fortran, MPI, and others. The recipes drive
+the individual packages build systems and manage packages they depend on.


Suggested change

the individual packages build systems and manage packages they depend on.

the individual packages' build systems and manage packages they depend on.

package, plural, possessive

Thanks. Fixed.

agcapps · 2024-10-02T21:50:23Z

src/docs/sphinx/dev_guide/updating_tpls.rst

 Spack also handles system level packages, so you can describe where they are on your
-system instead of building them from scratch.  You will need to describe which compilers
+system instead of building them from scratch. You will need to describe which compilers


What? Only single-space between sentences? : )

agcapps · 2024-10-02T21:57:15Z

src/docs/sphinx/dev_guide/updating_tpls.rst

+  We try to minimize these, but we often have to alter the existing Spack packages to: apply fixes
+  before pushing them up to Spack proper, or alter recipes in ways that are Axom specific.
+  Such overrides do not happen at the Spack level, but at the next level, Uberenv 
+  described below.


I suggest:

remove the colon from "to: apply fixes"

remove the comma from "up to Spack proper, or alter recipes"

either add a comma, making it "the next level, Uberenv, described below," or add parentheses, making it "the next level, Uberenv (described below)."

The last point is needed because "described below" is an adjective phrase modifying "Uberenv." Without the comma or the parens, overrides happen at the noun phrase "Uberenv described below" which makes less sense.

Got it. Change made.

agcapps · 2024-10-02T22:11:24Z

src/docs/sphinx/dev_guide/updating_tpls.rst

+  copy of the Axom repository in which the script is run. After building all of TPL sets
+  for a platform, it will test Axom against those TPL installs as well as the ``using-with-cmake``
  example for correctness. This script stops at the first failed TPL build but


Suggested change

copy of the Axom repository in which the script is run. After building all of TPL sets

for a platform, it will test Axom against those TPL installs as well as the ``using-with-cmake``

example for correctness. This script stops at the first failed TPL build but

copy of the Axom repository in which the script is run. After building all TPL sets

for a platform, it will attempt to build Axom and the ``using-with-cmake`` example against

each set of TPLs. This script stops at the first failed TPL build but

Recommended change pushed.

agcapps · 2024-10-02T22:12:49Z

src/docs/sphinx/dev_guide/updating_tpls.rst

-   to before it redirects the output from the command, i.e., ``[[log file: /path/to/log``.
+   Due to the large amount of information printed to the screen during a full build, the build scripts
+   redirect most build step output to log files. This output will tell you what command is being run,
+   i.e., ``[EOE: some/command --with-options]``, and will tell you the log file being written


Good lord. What was I doing?!?

Fixed.

agcapps · 2024-10-02T22:33:01Z

src/docs/sphinx/dev_guide/updating_tpls.rst


 #. Download the new release and override the source that is already there.
-   This can often involve removing files no-longer needed but most of the
-   current ones are a single header file.
+   This may involve removing files no-longer needed.


Suggested change

This may involve removing files no-longer needed.

This may involve removing files no longer needed.

(minor suggestion for style and clarity)

agcapps · 2024-10-02T23:01:48Z

src/docs/sphinx/dev_guide/updating_tpls.rst

 with a new set of TPLs. Typically, this process is followed when you want to 
-update one or more TPLs which Axom depends on. After they are built and
-the required changes are merged into develop, they will be available for
+update one or more TPLs on which Axom depends. After they are built and


Possibly:

Suggested change

update one or more TPLs on which Axom depends. After they are built and

update one or more TPLs. After they are built and

They're our TPLs. We don't have to say again that we depend on them.

Thank you. Fixed.

agcapps

Thanks, Rich! Much clearer.

kennyweiss

Thanks @rhornung67 for this thorough update to our build and release process!

kennyweiss · 2024-10-02T23:08:11Z

src/docs/sphinx/dev_guide/updating_tpls.rst

+  the end indicating which Axom build succeeded or failed.
+* ``build_src.py``: This script uses the existing host-configs in your local clone of the
+  Axom repo, or a specific one you point at, and builds and tests Axom against them. It also
+  tests the ``using-with-cmake`` example.


Possibly helpful clarification:

Suggested change

tests the ``using-with-cmake`` example.

tests axom installation via the ``using-with-cmake`` example and axom tutorials.

Got it. Thank you.

kennyweiss · 2024-10-02T23:11:08Z

src/docs/sphinx/dev_guide/updating_tpls.rst

+   .. important:: To install TPL builds to be shared by all Axom developers and used 
+                  in our GitLab CI, you will need to become the Axom service user ``atk``.
+                  There is a clone of the Axom repo in the ``/usr/workspace/atk/axom_repo``
+                  directory. After becoming ``atk``, you can go into that directory and
+                  switch to the branch you made to test your changes. Before running the
+                  TPL builds, make sure the branch is updated, including all submodules.


Should we mention that this is only applicable to the subset of axom developers who can become atk ?

I think that's implied since the /usr/workspace/atk directory is not accessible if you do not xsu atk and only members of the team can do that.

The main point I was trying to make is that you really need to become atk and use that clone of the repo to build and install the shared TPLs.

rhornung67 · 2024-10-03T18:50:34Z

I've addressed all the reviewer comments so far. Thank you for the corrections and suggestions. If anyone else wants to review this, please do that soon. I will merge it when all checks are green.

rhornung67 added 10 commits September 20, 2024 16:06

Freshen up basic descriptions of TPL things

abbca4d

Freshen up more TPL update documentation

718bf07

Merge branch 'develop' into task/rhornung67/devguide-tpls

b280b73

Add more description to release process section

bf753b4

Simplify text and fix formatting

90b2c6f

Move comment out of list so it shows up on RTD.

493dd1f

Fix note syntax

26abd66

Fix comment about squash merging

2533874

Merge branch 'develop' into task/rhornung67/devguide-tpls

6dafc4e

Add more details to TPL discussion

2b2d440

rhornung67 changed the title ~~Task/rhornung67/devguide tpls~~ Axom Dev Guide Updates Sep 27, 2024

rhornung67 requested review from white238, BradWhitlock, kennyweiss, nselliott, ltaylor16, gunney1, agcapps and bmhan12 September 27, 2024 22:26

Merge branch 'develop' into task/rhornung67/devguide-tpls

de82ca8

bmhan12 approved these changes Oct 2, 2024

View reviewed changes

Fixed typo caught by reviewer

d9c3a6e

agcapps reviewed Oct 2, 2024

View reviewed changes

rhornung67 added 2 commits October 2, 2024 15:50

Fixes from reviewer comments

dbef3e1

Merge branch 'develop' into task/rhornung67/devguide-tpls

0c76c00

agcapps reviewed Oct 2, 2024

View reviewed changes

agcapps approved these changes Oct 2, 2024

View reviewed changes

kennyweiss approved these changes Oct 2, 2024

View reviewed changes

Changes suggested by reviewers

31d27bd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Axom Dev Guide Updates #1430

Axom Dev Guide Updates #1430

rhornung67 commented Sep 27, 2024 •

edited

Loading

bmhan12 Oct 2, 2024

rhornung67 Oct 2, 2024

agcapps Oct 2, 2024

rhornung67 Oct 2, 2024

agcapps Oct 2, 2024

agcapps Oct 2, 2024

rhornung67 Oct 2, 2024

agcapps Oct 2, 2024

rhornung67 Oct 2, 2024

agcapps Oct 2, 2024

rhornung67 Oct 2, 2024 •

edited

Loading

agcapps Oct 2, 2024

rhornung67 Oct 2, 2024

agcapps Oct 2, 2024

rhornung67 Oct 3, 2024

agcapps left a comment

kennyweiss left a comment

kennyweiss Oct 2, 2024

rhornung67 Oct 3, 2024

kennyweiss Oct 2, 2024

rhornung67 Oct 3, 2024

rhornung67 commented Oct 3, 2024

	regular development. After the bus is resolved and that pull request is
	regular development. After the bug is resolved and that pull request is

	the individual packages build systems and manage packages they depend on.
	the individual packages' build systems and manage packages they depend on.

	This may involve removing files no-longer needed.
	This may involve removing files no longer needed.

	update one or more TPLs on which Axom depends. After they are built and
	update one or more TPLs. After they are built and

	tests the ``using-with-cmake`` example.
	tests axom installation via the ``using-with-cmake`` example and axom tutorials.

Axom Dev Guide Updates #1430

Are you sure you want to change the base?

Axom Dev Guide Updates #1430

Conversation

rhornung67 commented Sep 27, 2024 • edited Loading

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhornung67 Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agcapps left a comment

Choose a reason for hiding this comment

kennyweiss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhornung67 commented Oct 3, 2024

rhornung67 commented Sep 27, 2024 •

edited

Loading

rhornung67 Oct 2, 2024 •

edited

Loading