Skip to content

DRAFT: Config loading #35

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

DRAFT: Config loading #35

wants to merge 13 commits into from

Conversation

machshev
Copy link
Collaborator

@machshev machshev commented Oct 1, 2025 •
edited
Loading

Significant improvement in config loading mechanism.

  • Pull out the Project meta data into a separate module - it's currently conflated with the HJSON config
  • Add tests to the HJSON config loading functions and refactor to start tidying it up a bit.
  • Update the flow config classes to make use of these improved mechanisms
  • Resolve the evaluation order issues

Note: there is a known issue in this code at the moment due to the non-commutative nature of the wildcard substitution. Specifically with the eval_cmd mechanism (see #17 for more information). Before this is merged it needs to be checked with something like #34.

@machshev machshev force-pushed the config_loading branch 5 times, most recently from cd27047 to 62085bf Compare October 3, 2025 15:10
@machshev
feat: add deployment object dump debug feature
61776ff 
Debug feature to dump a JSON representation of the `Deploy` objects,
just before they are sent to the scheduler. This feature is hidden behind
an environment variable to avoid cluttering the CLI interface. The full
`Deploy` object is not JSON serialisable in it's current form, so we
only serialise the attributes that the `LocalLauncher` uses and some
identifying attributes like the full name of the job and the deployment
class name.

To enable set `DVSIM_DEPLOY_DUMP=true`, run DVSim and there will be a
file called `deploy-<branch>-<timestamp>.json` generated in the scratch
directory `scratch/deploy-<branch>-<timestamp.json>`.

This file can they be compared between runs to check for regressions in
the flow leading up to job deployment. With `--fixed-seed=1 --branch=baseline`,
the resulting json files should be identical. Setting the "branch"
doesn't actually change the git branch, it just overrides what DVSim
thinks is the branch as far as paths in the scratch directory are
concerned. We need to either set this to something fixed so that the
deployment objects contain the same paths, otherwise a diff will fail.

Generating a `diff` or hash of the two files and comparing shows if the
job deployments would be identical or not.

Using this functionality I have confirmed, by backporting this commit to
the first release tag, that none of the refactorings made so far in this
repository have changed the deployment objects in such a way that the
launched jobs are different in any way.

```
✦ ❯ sha512sum baseline.json deploy_25.10.03_11.35.33.json
af732c3011753cfc7dc40068e1ce9b6cf3624973ffbbd25f828e4d7727f27dd65b5ada19407500315ea6f279946222d185dc939fe4978b0b32b3e7e8b4f7f60c  baseline.json
af732c3011753cfc7dc40068e1ce9b6cf3624973ffbbd25f828e4d7727f27dd65b5ada19407500315ea6f279946222d185dc939fe4978b0b32b3e7e8b4f7f60c  deploy_25.10.03_11.35.33.json
```

The first JSON file was generated from backporting the this feature to
the tagged commit. The second file was generated with this branch and
includes all the tidyup work made so far to DVSim.

The DVSim command used is:

```
DVSIM_DEPLOY_DUMP=true dvsim \
    ~/base/opentitan/hw/top_earlgrey/dv/top_earlgrey_sim_cfgs.hjson \
    --proj-root ~/base/opentitan -i nightly --scratch ~/scratch \
    --branch baseline --fixed-seed 1 \
    --verbose=debug --max-parallel=50 \
    --cov
```

**Note:** *your hashes will be different from mine as the directory paths
will be different. However if you run this against the same versions
your hashes should be the same as each other.*

**Note:** *Depending on the flags you pass to DVSim there may be some
minor indeterminism in the configuration. For example with CovMerge the
coverage directories are not always given in the same order. It can take
several runs to get a different hash, or it could be different on every
run. In such cases it's worth using `diff` across the files to see what
the actual differences are, they may not be consequential.*

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
fix: results refactor name clash
ca9405a 
Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
feat: [git] add repo root helper and migrate to GitPython library
5b95456 
With the nightly regression running functionality more git interaction
is required. Rather than implementing everything from scratch using
subprocess calls and maintaining it, add the GitPython library as a
dependency and use that for the git interactions.

The repo root helper is implemented using Pathlib as the GitPython
library doesn't seem to support this functionality. It requires a path
to the repo root exactly and fails if it doesn't get a valid repo path.

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: [project] move out the project level config functions.
b4c99b0 
The cli module should probably be a thin wrapper around
internal functions rather than containing a fairly large
amount of business logic. This commit copies out and
improves this logic.

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: [config] improved config loading with testing
0e0862f 
Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: [config] update cli and flow config classes to use the new …
d81f214 
…config loader.

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: [config] save the config to JSON file in the scratch dir
9e4b6cd 
Currently there is no tracability in terms of what config was actually used to
produce the generated results. This commit adds serialisation to save
the config file to the root of the scratch dir for later reference.

At some point this could be used to restart or finish off a job that
failed for some reason.

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: [project] rename ProjectMeta to Project
e1ce0ba 
Given the ProjectMeta object is becoming more than
just meta data.

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: [config] move the flow config into the project config
c19c454 
This means the project config contains the whole project
configuration including the flow config. This can be broken
down later into smaller Pydantic objects as the fields and
types become better defined.

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: small tidy up refactor
c6d9d7f 
Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
fix: [config] add explicit relative path field to the config loading
973b201 
The rel_path is a calculated field that is critical to determining where
files are placed and found in the scratch dir and also the reports dir.
At the moment the field is not created until the flow objects are
created. Which means that the saved config doesn't have the information
unless it's overridden in the config itself.

This commit moves the calculated field out of the flow base class init
into the config loading stage where it is saved the filesystem in the
scratch dir along with the rest of the loaded config. Which means it can
be used to navigate the scratch dir after a run.

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: [config] move overrides from flow base to config loading stage
53246fd 
This is to ensure that the saved project config is already resolved
otherwise the overrides mechanism would need to be implemented in
any code that loads the project config.

Without this change, the report generation code can't see the correct
rel_path field as it is overridden in the flow object after being saved
as part of the project config.

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
refactor: [config] resolve wildcards on load
2113e81 
Resolve as many wildcards as possible on config load. Now that the overrides
have moved back into the config loading stage the overridden fields are not
having their wildcards resolved for some reason.

It is useful to do this anyway as we then have more resolved config values
in the saved project config

Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
@machshev
Copy link
Collaborator Author

machshev commented Oct 7, 2025

Using #39 the divergence using this approach is clear. The issue is around the wildcard evaluation timing. This PR attempts to evaluate the wildcards as soon as possible in the flow, however because of the object model within DVSim this results in substituting the uvm_test_seq with the default value instead of the per test test sequence.

[-+UVM_TEST_SEQ=adc_ctrl_common_vseq-]{++UVM_TEST_SEQ=adc_ctrl_base_vseq+}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@machshev