Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Feb 10, 2026

Description

Consolidates configuration headers from ./setup into core/specfem, removes unused default configuration system, and eliminates deprecated code patterns. Includes merge of latest devel branch changes to ensure all include paths are updated comprehensively.

Key Changes

  • Relocated setup files: setup/specfem_setup.hpp.incore/specfem/setup.hpp.in, setup/constants.hpp.incore/specfem/constants.hpp (converted to plain header - no CMake variables in use)
  • Removed default config system: Eliminated __default_file__, parameter_files/defaults/default.yaml, and fallback logic in runtime configuration. setup constructor now takes single parameter file.
  • Cleaned up unused code: Removed globals.h (contained unused simulation_wave variable), removed unused build_configuration namespace (contained hardcoded chunk_size, num_chunks, num_threads, vector_lanes constants), confirmed specfem::wave enum was already absent
  • Namespace refactoring: constants::empiricalconstants, moved internal program_2d/3d functions to detail namespace
  • Compile-time improvements: Changed pi from runtime computation to constexpr literal
  • Fixed include paths: Updated all include paths from "constants.hpp" to "specfem/constants.hpp" and from "specfem_setup.hpp" to "specfem/setup.hpp" across the entire codebase including:
    • All .tpp files in the attenuation module (compute_tau_eps.tpp, compute_tau_sigma.tpp, compute_factors.tpp)
    • All .hpp files in the attenuation module (compute_factors.hpp)
    • Merged latest devel branch changes that added new attenuation files and updated their include paths

Updated Signatures

// Before
setup(const std::string &parameter_file, const std::string &default_file);

// After  
setup(const std::string &parameter_file);

All includes updated: "specfem_setup.hpp""specfem/setup.hpp", "constants.hpp""specfem/constants.hpp"

Issue Number

Related to #1626

Checklist

  • I ran the code through pre-commit to check style
  • THE DOCUMENTATION BUILDS WITHOUT WARNINGS/ERRORS
  • I have added labels to the PR (see right hand side of the PR page)
  • My code passes all the integration tests
  • I have added sufficient unittests to test my changes
  • I have added/updated documentation for the changes I am proposing
  • I have updated CMakeLists to ensure my code builds
  • My code builds across all platforms
Original prompt

This section details on the original issue you should resolve

<issue_title>Deprecate ./setup folder</issue_title>
<issue_description>## Task description

Write a list of things that need to be completed to achieve the goal of this issue.

  • specfem_setup.hpp.in -> core/specfem/setup.hpp.in
  • constants.hpp.in -> core/specfem/constants.hpp, should be plain hpp not configure file, since the configure variables are not being used.
  • deprecate globals.hpp, None of the variables are being used

The configuration for setup.hpp.in should go into the cmakelists in core/specfem. The configured file should be into the configured file into /include/specfem/setup.hpp

Acceptance criteria

  • project builds with new configuration and deprecated setup
    </issue_description>

<agent_instructions>- You can remove all backwards capability for the default config since it is not used anyways and should fail tests if something unpredicted depends on it! So please remove parameter_files parameter_files/defaults parameter_files/defaults/default.yaml as well

  • constants::empirical should be renamed to constants only.
  • remove specfem::enum::wave from constants since unused

This is a redo of #1626 since there were too many merge conflicts</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@buildbot-princeton
Copy link
Collaborator

Can one of the admins verify this patch?

2 similar comments
@buildbot-princeton
Copy link
Collaborator

Can one of the admins verify this patch?

@buildbot-princeton
Copy link
Collaborator

Can one of the admins verify this patch?

@lsawade lsawade requested a review from icui February 10, 2026 14:25
Copilot AI changed the title [WIP] Deprecate ./setup folder and update configuration Deprecate ./setup folder and refactor configuration structure Feb 10, 2026
Copilot AI requested a review from lsawade February 10, 2026 14:38
@lsawade
Copy link
Collaborator

lsawade commented Feb 10, 2026

test this please

} // namespace specfem

namespace specfem {
namespace build_configuration {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the unused build_configuration namespace in commit 6e37b37.

@lsawade lsawade marked this pull request as ready for review February 10, 2026 16:20
@lsawade
Copy link
Collaborator

lsawade commented Feb 10, 2026

@copilot make sure to check the workflows, there seem to be some include errors

Copy link
Contributor Author

Copilot AI commented Feb 10, 2026

@copilot make sure to check the workflows, there seem to be some include errors

Fixed the include errors in core/specfem/attenuation/compute_tau_eps.tpp and core/specfem/attenuation/compute_tau_sigma.tpp. These files had old #include "constants.hpp" paths that needed to be updated to #include "specfem/constants.hpp". Fixed in commit be49229.

@lsawade
Copy link
Collaborator

lsawade commented Feb 11, 2026

@copilot tests are still failing

In file included from /home/runner/work/SPECFEMPP/SPECFEMPP/core/specfem/attenuation.hpp:7,
                 from /home/runner/work/SPECFEMPP/SPECFEMPP/tests/unit-tests/attenuation/compute_tau_sigma_tests.cpp:1:
/home/runner/work/SPECFEMPP/SPECFEMPP/core/specfem/attenuation/compute_factors.hpp:3:10: fatal error: constants.hpp: No such file or directory
    3 | #include "constants.hpp"
      |          ^~~~~~~~~~~~~~~
compilation terminated.
gmake[2]: *** [tests/unit-tests/CMakeFiles/attenuation_tests.dir/build.make:93: tests/unit-tests/CMakeFiles/attenuation_tests.dir/attenuation/compute_tau_sigma_tests.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:7800: tests/unit-tests/CMakeFiles/attenuation_tests.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....

Make sure you check the entire code base for #include "constants.hpp" and update all references to #include "specfem/constants.hpp"

@lsawade lsawade removed the request for review from icui February 11, 2026 15:01
Copy link
Contributor Author

Copilot AI commented Feb 11, 2026

@copilot tests are still failing

...

Fixed all remaining include errors. The issue was that new files (compute_factors.hpp and compute_factors.tpp) were added to the devel branch after my branch was created. I merged the latest devel changes and updated those files to use the correct include paths: #include "specfem/constants.hpp" and #include "specfem/setup.hpp". Fixed in commit f20d9ee.

@lsawade lsawade closed this Feb 11, 2026
@lsawade lsawade deleted the copilot/deprecate-setup-folder-another-one branch February 11, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants