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.

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

90 allow api users to create and destroy instances of tuvx #96

Merged
merged 46 commits into from May 1, 2024
Merged

90 allow api users to create and destroy instances of tuvx #96

merged 46 commits into from May 1, 2024

Conversation

K20shores
Copy link
Collaborator

@K20shores K20shores commented Apr 25, 2024 •
edited

Closes #90

  • adds a musica namespace to all of our files
  • combined the two fortran test cases of the micm api into one file
  • adds an external facing C api to create and destory TUVX
  • adds an internal facing C api to create an destroy TUVX with a corresponding fortran interface. This will allow us to transition parts of tuvx into C/C++ without changes needing to propagate up to cam-sima
  • changed the name of the micm module from micm_core to musica_micm_core. This is because tuvx_core already exists in tuvx and to have consistent naming in the musica fortran library something needed to change

@K20shores K20shores linked an issue Apr 25, 2024 that may be closed by this pull request
Allow API users to create and destroy instances of TUV-x #90
Closed
@K20shores K20shores marked this pull request as ready for review April 25, 2024 17:22
@K20shores K20shores changed the title 90 allow api users to create and destroy instances of tuv x 90 allow api users to create and destroy instances of tuvx Apr 25, 2024
boulderdaze
boulderdaze requested changes Apr 26, 2024
View reviewed changes
Copy link
Collaborator

@boulderdaze boulderdaze left a comment

Choose a reason for hiding this comment

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

Looks good! Just have some hopefully minor change requests.

include/musica/tuvx.hpp Outdated Show resolved Hide resolved
include/musica/version.h Outdated Show resolved Hide resolved
src/tuvx/tuvx.cpp Outdated Show resolved Hide resolved
include/musica/tuvx.hpp Show resolved Hide resolved
src/tuvx/tuvx.cpp Show resolved Hide resolved
fortran/tuvx_core.F90 Outdated Show resolved Hide resolved
fortran/test/fetch_content_integration/test_tuvx_api.F90 Outdated Show resolved Hide resolved
mattldawson
mattldawson approved these changes Apr 26, 2024
View reviewed changes
Copy link
Collaborator

@mattldawson mattldawson left a comment

Choose a reason for hiding this comment

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

looks great to me! Just one question about a header file. I'm not sure if it's a problem or not if it builds ok, but see what you think

include/musica/util.hpp Show resolved Hide resolved
K20shores and others added 4 commits April 30, 2024 11:02
@K20shores
resolving typos, incorrect comments
d95a954
@K20shores @boulderdaze
Update fortran/tuvx_core.F90
9dd03d7 
Co-authored-by: Jiwon Gim <55209567+boulderdaze@users.noreply.github.com>
@K20shores @boulderdaze
Update fortran/test/fetch_content_integration/test_tuvx_api.F90
bee0666 
Co-authored-by: Jiwon Gim <55209567+boulderdaze@users.noreply.github.com>
@K20shores
correcting placement of ifdef for error functions
1add8a5
@K20shores K20shores requested a review from boulderdaze May 1, 2024 13:43
boulderdaze
boulderdaze approved these changes May 1, 2024
View reviewed changes
Copy link
Collaborator

@boulderdaze boulderdaze left a comment

Choose a reason for hiding this comment

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

Thank you for addressing the comments! Could you also remove MICM comments in the src/tuvx/tuvx.cpp? I don't need to review for that.

@K20shores K20shores merged commit a735c3e into main May 1, 2024
40 checks passed
@K20shores K20shores deleted the 90-allow-api-users-to-create-and-destroy-instances-of-tuv-x branch May 1, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattldawson mattldawson mattldawson approved these changes

@boulderdaze boulderdaze boulderdaze approved these changes

@WardF WardF Awaiting requested review from WardF

@mwaxmonsky mwaxmonsky Awaiting requested review from mwaxmonsky

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow API users to create and destroy instances of TUV-x
3 participants
@K20shores @mattldawson @boulderdaze