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

Tpetra: Add a way to query environment variables #654

Closed
mhoemmen opened this issue Sep 26, 2016 · 9 comments
Closed

Tpetra: Add a way to query environment variables #654

mhoemmen opened this issue Sep 26, 2016 · 9 comments
Assignees

Comments

@mhoemmen
Copy link
Contributor

@trilinos/tpetra

Add a way for Tpetra to query environment variables. This would be useful for making more debug options available at run time, so that users wouldn't have to recompile their release build of Trilinos in order to get some useful debug information. It would also be useful for debugging issues like #423, which relate to downstream packages or applications making incorrect assumptions (e.g., that stride == local # rows, which is not true if padding is enabled).

It may make sense to cache results of looking up an environment variable, in order to avoid overhead of system calls.

It would make sense to design this as a singleton class. This would make it easier to ensure thread safety by locking use of getenv(). POSIX getenv() does not promise thread safety:

http://pubs.opengroup.org/onlinepubs/009695399/functions/getenv.html

though Linux's implementation of getenv() does promise thread safety:

http://man7.org/linux/man-pages/man3/getenv.3.html

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Sep 26, 2016

Here is the interface I would like (omitting details relating to singleton initialization and finalization; use atexit() hook to finalize). Put in a new header file called Tpetra_Details_Environment.hpp in tpetra/core/src.

namespace Tpetra {
namespace Details {

class Environment {
public:
  //! Get singleton instance
  static Environment& getInstance ();

  /// \brief Does the given variable exist?
  ///
  /// This is nonconst because the implementation
  /// reserves the right to cache results.
  bool variableExists (const std::string& variableName);

  /// \brief Get value of the given variable.
  ///
  /// This is nonconst because the implementation
  /// reserves the right to cache results.
  ///
  /// \return If the variable does not exist in the environment,
  ///   return "".  Else, return the variable's value, as a string.
  std::string getValue (const std::string& variableName);

private:
  //! The singleton instance
  static Environment* theInstance_;
};

} // namespace Details
} // namespace Tpetra

@srajama1
Copy link
Contributor

Thinking out loud : Can this be in Trilinos name space and in a place like Teuchos ? Seems much more useful at the larger scale.

@krcb
Copy link
Contributor

krcb commented Sep 26, 2016

Could std::getenv be of use here? Since CXX11 this function is thread-safe as long as no other function modifies the environment:

http://en.cppreference.com/w/cpp/utility/program/getenv

@mhoemmen
Copy link
Contributor Author

@srajama1 wrote:

Thinking out loud : Can this be in Trilinos name space and in a place like Teuchos ? Seems much more useful at the larger scale.

We could test in Tpetra first, then deploy in Teuchos once we're confident that it works. Breaking Teuchos is even higher consequence than breaking Tpetra :-)

@mhoemmen
Copy link
Contributor Author

@krcb wrote:

Could std::getenv be of use here? Since CXX11 this function is thread-safe as long as no other function modifies the environment . . .

Yes; thanks!

@mhoemmen mhoemmen added the stage: in progress Work on the issue has started label Oct 4, 2016
@mhoemmen
Copy link
Contributor Author

mhoemmen commented Oct 4, 2016

Progress report: @tjfulle wrote an initial implementation and stand-alone test. I reviewed the implementation and gave some comments on refining the interface. Next step after that is to integrate this into Trilinos' unit test framework.

@tjfulle
Copy link
Contributor

tjfulle commented Oct 10, 2016

Commit 347c81a of my fork of Trilinos adds this feature. @mhoemmen, do you want to take a look at it before I put through an official pull request?

@mhoemmen
Copy link
Contributor Author

@tjfulle wrote:

@mhoemmen, do you want to take a look at it before I put through an official pull request?

I just added some comments to that commit. Thanks!

mhoemmen pushed a commit that referenced this issue Oct 26, 2016
Adds Tpetra::Details::Environment singleton class:
- query system for environment variables
- cache retrieved environment variables
- cache default variables on instantiation (TPETRA_DEBUG,
  TPETRA_USE_BLAS)
- setting environment variables capability is disabled

Commit also provides an updated method of loading configuration files
with the checkin-test.py script, allowing configuration files to have
extensions different than '.py'

Addresses: #654, #684, #688

Build/Test Cases Summary
Enabled Packages: TpetraCore, Ifpack2, Belos, Amesos2, Zoltan2, Xpetra, MueLu, Stokhos, Sacado
Disabled Packages: SEACAS,STK,Shards,FEI,ROL,Moertel,Panzer,ThreadPool,OptiPack,Rythmos,Intrepid,PyTrilinos
0) MPI_DEBUG => passed: passed=515,notpassed=0 (11.04 min)
@mhoemmen mhoemmen added this to the Tpetra-backlog milestone Nov 2, 2016
@mhoemmen
Copy link
Contributor Author

mhoemmen commented Nov 2, 2016

I'm calling this closed. Thanks @tjfulle !

@mhoemmen mhoemmen closed this as completed Nov 2, 2016
@mhoemmen mhoemmen removed the stage: in progress Work on the issue has started label Nov 2, 2016
@mhoemmen mhoemmen removed this from the Tpetra-backlog milestone Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants