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

Convert string-param parsing #419

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

mabruzzo
Copy link
Collaborator

Description

This patch relocates all of the string parameter parsing from Old_Style_Parse_Param to Init_Param_Struct_Members. The relocated logic was converted to use the new-style of parameter parsing.

  • While I was doing this, I relocated (& converted) some additional parameter-parsing logic. Essentially I did this when a string parameter was part of a small group of related parameter-logic and I wanted to avoid breaking up this group.
  • Specifically, the non-string logic that was relocated includes Cosmology parameters and ANALYSIS parameters (more on this below)

Motivation

The primary motivation is so that we can make the shift to syntactic typing of parameters. I've explained what this is other PRs and this parsing logic conversion needs to be made sooner or later, so I won't go into much detail.

The main reason for this PR is that I think we should make the shift to syntactic typing sooner rather than later.

  • Essentially, this shift just means that string-parameter values will need to be quoted.
  • I think this change will need to be gradually phased in (since older parameter files will need to be converted).
  • It would be useful to start this process sooner rather than later so that new parameter files can use syntactic typing (the longer we wait, the more conversion work that will be needed later).

Once we fully adopt syntactic typing Cholla parameter files will be fully consistent with the TOML parameter file format (it will be a strict subset). This means we could eventually replace our custom parsing logic with logic from an existing toml parsing library (there are some nice header-only libraries that we could easily drop into cholla). Using a library introduces a number of benefits (e.g. we could support parameters that specify a list of values, which is extremely useful). There are also some other benefits, too.

Relocated Cosmology and Analysis Parameters

I took a lot of care to ensure that the parsing logic remains consistent.

  • Historically, cosmological parameters had no default values (and minimal value sanitization). Essentially there was not error checking. Now, cosmological simulations automatically raise errors if a user forgets to specify any cosmological parameter. The lone exception is the wa parameter which now defaults to a value of 0 if omitted (i.e. consistent with LCDM cosmology)
  • The analysis logic previously didn't have any default values. To avoid breaking Cholla with existing parameter-files, I made the parsing logic default to dummy values in most cases (so effectively, parsing of these parameters is as permissive as ever). The exception is the skewersdir parameter. Cholla will abort with an error if the user forgets this parameter in a simulation compiled with -DOUTPUT_SKEWERS

Description
-----------

This patch relocates all of the string parameter parsing from
``Old_Style_Parse_Param`` to ``Init_Param_Struct_Members``. The
relocated logic was converted to use the new-style of parameter
parsing.
- While I was doing this, I relocated (& converted) some additional
  parameter-parsing logic. Essentially I did this when a string
  parameter was part of a small group of related parameter-logic and I
  wanted to avoid breaking up this group.
- Specifically, the non-string logic that was relocated includes
  Cosmology parameters and ANALYSIS parameters (more on this below)

Motivation
----------

The primary motivation is so that we can make the shift to syntactic
typing of parameters. I've explained what this is other PRs and this
parsing logic conversion needs to be made sooner or later, so I won't go
into much detail.

The main reason is that I think we should make the shift sooner rather
than later (essentially all strings will need to be quoted). I think
this will need to be gradually phased in (since older parameter files
will need to be converted). It would be useful to start this process
sooner rather than later so that new parameter files can use syntactic
typing (the longer we wait, the more conversion work that will be needed
later).

Once we fully adopt syntactic typing Cholla parameter files will be
fully consistent with the TOML parameter file format. This means we
could eventually replace our custom parsing logic with logic from an
existing toml parsing library (this brings a number of benefits).

Relocated Cosmology and Analysis Parameters
-------------------------------------------

I took a lot of care to ensure that the parsing logic remains
consistent.
- Historically, cosmological parameters had no default values (and
  minimal value sanitization). Essentially there was not error checking.
  Now, cosmological simulations automatically raise errors if a user
  forgets to specify any cosmological parameter. The lone exception is
  the wa parameter which now defaults to a value of 0 if omitted (i.e.
  consistent with LCDM cosmology)
- The analysis logic previously didn't have any default values. To avoid
  breaking Cholla with existing parameter-files, I made the parsing
  logic default to dummy values in most cases (so effectively, parsing
  of these parameters is as permissive as ever). The exception is the
  skewersdir parameter. Cholla will abort with an error if the user
  forgets this parameter in a simulation compiled with -DOUTPUT_SKEWERS
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.

1 participant