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

Add gsf module #303

Merged
merged 19 commits into from
Jun 4, 2024
Merged

Add gsf module #303

merged 19 commits into from
Jun 4, 2024

Conversation

lispandfound
Copy link
Contributor

@claudio525 Mentioned that the parsing code in ucgmsim/visualization#91 may have broader application and so should be included in qcore. I agree, so this pull request contains the GSF parsing code.

claudio525
claudio525 previously approved these changes May 13, 2024
@lispandfound
Copy link
Contributor Author

I have also added a new coordinate transformation module, and all the GSF related code from the type5 branch of Pre-processing. The coordinate transformation module is required for the GSF meshgrid generator. @claudio525 I know you approved the trivial parsing code, but it now includes quite a bit more. I will also add a description of the GSF file format to the wiki because I don't think it's documented anywhere.

Comment on lines 39 to 42
nztm_coords = np.array(
_WGS2NZTM.transform(wgs_depth_coordinates[:, 0], wgs_depth_coordinates[:, 1]),
).T
return np.append(nztm_coords, wgs_depth_coordinates[:, 2].reshape((-1, 1)), axis=-1)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be simplified to just
nztm_coords = np.array( _WGS2NZTM.transform(wgs_depth_coordinates[:, 0], wgs_depth_coordinates[:, 1], wgs_depth_coordinates[:, 2])).T

Copy link
Contributor

Choose a reason for hiding this comment

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

These comments would be nice to address?

qcore/coordinates.py Outdated Show resolved Hide resolved
qcore/gsf.py Show resolved Hide resolved
qcore/gsf.py Show resolved Hide resolved
qcore/gsf.py Outdated Show resolved Hide resolved
Copy link
Member

@sungeunbae sungeunbae left a comment

Choose a reason for hiding this comment

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

Happy in general - just a few comments

qcore/gsf.py Show resolved Hide resolved
qcore/gsf.py Show resolved Hide resolved
qcore/gsf.py Outdated Show resolved Hide resolved
@lispandfound
Copy link
Contributor Author

lispandfound commented May 14, 2024

@sungeunbae @joelridden I have addressed these concerns. I'll leave the unhappy parallel array gsf writing function for now but include a fault class into qcore that I can use in NSHMDB, Pre-Processing and any places later on where the database or type-5 code gets incorporated. That will be a separated pull request.

sungeunbae
sungeunbae previously approved these changes May 14, 2024
Copy link
Contributor

@joelridden joelridden left a comment

Choose a reason for hiding this comment

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

Just think the other 2 comments for the coordinates.py functions would be good to simplify

Comment on lines 39 to 42
nztm_coords = np.array(
_WGS2NZTM.transform(wgs_depth_coordinates[:, 0], wgs_depth_coordinates[:, 1]),
).T
return np.append(nztm_coords, wgs_depth_coordinates[:, 2].reshape((-1, 1)), axis=-1)
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments would be nice to address?

joelridden
joelridden previously approved these changes May 14, 2024
Copy link
Contributor

@claudio525 claudio525 left a comment

Choose a reason for hiding this comment

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

Couple of minor last minute comments. Otherwise looks great to me.

qcore/gsf.py Show resolved Hide resolved
qcore/gsf.py Show resolved Hide resolved
qcore/gsf.py Outdated Show resolved Hide resolved
qcore/coordinates.py Show resolved Hide resolved
qcore/coordinates.py Show resolved Hide resolved
qcore/gsf.py Outdated Show resolved Hide resolved
sungeunbae
sungeunbae previously approved these changes May 26, 2024
qcore/gsf.py Show resolved Hide resolved
Copy link
Contributor

@claudio525 claudio525 left a comment

Choose a reason for hiding this comment

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

Nice work!

@sungeunbae sungeunbae merged commit fdd11d7 into master Jun 4, 2024
1 check passed
@sungeunbae sungeunbae deleted the gsf_parsing branch June 4, 2024 22:11
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.

5 participants