Skip to content

Issue 1438 - Fixed parameter naming #1546

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

Merged
lsawade merged 4 commits into from
Jan 14, 2026
Merged

Issue 1438 - Fixed parameter naming #1546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26dcfcb
Merge branch 'devel' into issue-1438
lsawade Jan 6, 2026
0a15d46
Fixed parameter naming
lsawade Jan 7, 2026
013c110
Merge branch 'devel' into issue-1438
lsawade Jan 7, 2026
2bacb68
Merge branch 'devel' into issue-1438
lsawade Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions docs/sections/parameter_documentation/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ Parameter definitions
wavefield:
format: HDF5
directory: /path/to/output/folder
time-interval: 10

display:
format: PNG
Expand Down Expand Up @@ -424,12 +425,20 @@ Parameter definitions

:possible values: [string]

.. dropdown:: ``for_adjoint_simulations`` [optional]
.. dropdown:: ``time-interval`` [optional]

Time step interval for writing the wavefield.

:default value: 1 (-> every time step)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The documented default value for time-interval is "1 (-> every time step)" but the code returns 0 when the parameter is not provided. This inconsistency between documentation and implementation should be corrected. Either update the default value in the code to return 1, or update the documentation to reflect that the actual default is 0.

Suggested change
:default value: 1 (-> every time step)
:default value: 0

Copilot uses AI. Check for mistakes.

:possible values: [int]

.. dropdown:: ``include-last-step`` [optional]

Flag to indicate if the wavefield written with the intension to
be used in adjoint simulations.
Flag to indicate if the last time step should be included
when writing the wavefield.

:default value: False
:default value: True

:possible values: [bool]
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The parameter for-adjoint-simulations (renamed from for_adjoint_simulations) is missing from the documentation. This parameter is used in the code but has no corresponding documentation entry explaining its purpose, default value, and possible values. Documentation should be added for this parameter in the wavefield section.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The parameter time-interval-by-memory (renamed from time_interval_by_memory) is missing from the documentation. This parameter is used in the code but has no corresponding documentation entry explaining its purpose, default value, and possible values. Documentation should be added for this parameter in the wavefield section.

Copilot uses AI. Check for mistakes.

Expand Down
20 changes: 9 additions & 11 deletions src/parameter_parser/writer/wavefield.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,37 +36,37 @@ specfem::runtime_configuration::wavefield::wavefield(
}

const int time_interval = [&]() -> int {
if (Node["time_interval"]) {
return Node["time_interval"].as<int>();
if (Node["time-interval"]) {
return Node["time-interval"].as<int>();
} else {
return 0;
}
}();

const std::string time_interval_by_memory = [&]() -> std::string {
if (Node["time_interval_by_memory"]) {
if (Node["time-interval-by-memory"]) {
if (time_interval != 0) {
throw std::runtime_error(
"time_interval and time_interval_by_memory cannot be used "
"time-interval and time-interval-by-memory cannot be used "
"simultaneously");
}
return Node["time_interval_by_memory"].as<std::string>();
return Node["time-interval-by-memory"].as<std::string>();
} else {
return "";
}
}();

const bool include_last_step = [&]() -> bool {
if (Node["include_last_step"]) {
return Node["include_last_step"].as<bool>();
if (Node["include-last-step"]) {
return Node["include-last-step"].as<bool>();
} else {
return true;
}
}();

const bool for_adjoint_simulations = [&]() -> bool {
if (Node["for_adjoint_simulations"]) {
return Node["for_adjoint_simulations"].as<bool>();
if (Node["for-adjoint-simulations"]) {
return Node["for-adjoint-simulations"].as<bool>();
} else {
return false;
}
Expand Down Expand Up @@ -180,8 +180,6 @@ template std::shared_ptr<
specfem::runtime_configuration::wavefield::instantiate_wavefield_writer<
specfem::dimension::type::dim2>() const;



template std::shared_ptr<
specfem::periodic_tasks::periodic_task<specfem::dimension::type::dim2> >
specfem::runtime_configuration::wavefield::instantiate_wavefield_reader<
Expand Down