Skip to content

Conversation

lsawade
Copy link
Collaborator

@lsawade lsawade commented Sep 16, 2025

Description

This PR

  • Updates the io/sources tests to include source file and yaml node reading.
  • renames read_receivers -> read_2d_receivers
  • adds read_3d_receivers
  • adds IO tests for STATIONS file reading and yaml node reading for receivers

Issue Number

Closes #1211

Checklist

Please make sure to check developer documentation on specfem docs.

  • 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

@lsawade lsawade added the enhancement New feature or request label Sep 16, 2025
Copy link
Collaborator

@Rohit-Kakodkar Rohit-Kakodkar left a comment

Choose a reason for hiding this comment

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

Minor comments.

Comment on lines +25 to +28
specfem::utilities::is_close(this->x, other.x) &&
specfem::utilities::is_close(this->z, other.z) &&
specfem::utilities::is_close(this->angle, other.angle);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can 2 receivers have same station and network name but have different locations/angle? Should this be a assert?

Copy link
Collaborator Author

@lsawade lsawade Sep 17, 2025

Choose a reason for hiding this comment

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

This is really only used for checking the read-in stations in the unit tests, I'm not sure whether there is a different use case. If you just want to check whether receivers share the same station name or just the same location, the approach should be slightly different maybe implementing a new a method like same_code(other) or same_location(other)

We could implement a function that weeds out potential duplicates, or throws an error if a vector of receivers contains two receivers of the same angle and location.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No I am wondering we should throw a runtime error if stations have same name but different location/angle. It's more to do with symantics of operator==. For example, what would be the expected behavior when someone uses the function outside of test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, understood! Yes i agree.

Comment on lines +22 to +29
bool specfem::receivers::receiver<specfem::dimension::type::dim3>::operator==(
const receiver &other) const {
return (this->network_name == other.network_name) &&
(this->station_name == other.station_name) &&
specfem::utilities::is_close(this->x, other.x) &&
specfem::utilities::is_close(this->y, other.y) &&
specfem::utilities::is_close(this->z, other.z);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as 2D

@Rohit-Kakodkar Rohit-Kakodkar merged commit 3840e9a into devel Sep 19, 2025
8 checks passed
@Rohit-Kakodkar Rohit-Kakodkar deleted the issue-1211 branch September 19, 2025 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants