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

Enhance MET to compile and link against the Atlas and ecKit libraries #2574

Closed
8 of 21 tasks
JohnHalleyGotway opened this issue Jun 13, 2023 · 25 comments · Fixed by #2737
Closed
8 of 21 tasks

Enhance MET to compile and link against the Atlas and ecKit libraries #2574

JohnHalleyGotway opened this issue Jun 13, 2023 · 25 comments · Fixed by #2737
Assignees
Labels
component: build process Build process issue priority: blocker Blocker requestor: METplus Team METplus Development Team type: new feature Make it do something new
Milestone

Comments

@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Jun 13, 2023

Describe the New Feature

Adding support in MET for unstructured grids requires adding dependencies on the ecKit and/or Atlas libraries. These depend on the Proj library which is being added via MET#2669. This task is to install those libraries on the development machine, seneca, and update MET's configuration script and Makefiles to link to them. Basically, any Makefile currently linking to the vx_grid library should now also link to ecKit and/or Atlas.

Remember to update the development environment setup scripts in MET/internal/scripts/environment accordingly.

Recommend coordinating with @jprestop to discuss the implications on the compile_MET_all.sh script and the various supported platforms.

Recommend updating the User's Guide during this process as well to clarify the dependencies.

Acceptance Testing

List input data types and sources.
Describe tests required for new functionality.

Time Estimate

4 days?

Sub-Issues

Consider breaking the new feature down into sub-issues.
None

Relevant Deadlines

List relevant project deadlines here or state NONE.

Funding Source

7705991 MPAS Participation

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required
  • Select scientist(s) or no scientist required

Labels

  • Select component(s)
  • Select priority
  • Select requestor(s)

Projects and Milestone

  • Select Repository and/or Organization level Project(s) or add alert: NEED CYCLE ASSIGNMENT label
  • Select Milestone as the next official version or Future Versions

Define Related Issue(s)

Consider the impact to the other METplus components.

New Feature Checklist

See the METplus Workflow for details.

  • Complete the issue definition above, including the Time Estimate and Funding source.
  • Fork this repository or create a branch of develop.
    Branch name: feature_<Issue Number>_<Description>
  • Complete the development and test your changes.
  • Add/update log messages for easier debugging.
  • Add/update unit tests.
  • Add/update documentation.
  • Push local changes to GitHub.
  • Submit a pull request to merge into develop.
    Pull request: feature <Issue Number> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s) and Development issues
    Select: Repository level development cycle Project for the next official release
    Select: Milestone as the next official version
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added component: build process Build process issue type: new feature Make it do something new priority: blocker Blocker requestor: METplus Team METplus Development Team labels Jun 13, 2023
@JohnHalleyGotway JohnHalleyGotway added this to the MET 12.0.0 milestone Jun 13, 2023
@JohnHalleyGotway JohnHalleyGotway self-assigned this Jun 13, 2023
@hsoh-u hsoh-u self-assigned this Jul 7, 2023
@JohnHalleyGotway JohnHalleyGotway moved this from 🔖 Ready to 🏗 In progress in MET-12.0.0 Development Aug 23, 2023
@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Aug 23, 2023

Note, based on a UGRID project meeting on 8/23/23, it may be the case that only the eckit library is need and not Atlas. Atlas depends on eckit but @hsoh-u may only actually be using eckit features rather than Atlas-specific add ons. At this time, recommend compiling against Proj and eckit.

Note that ecKit contains tags (https://github.com/ecmwf/eckit/tags) but there are no corresponding releases created on GitHub. We'll need to note this detail in the METplus documentation.

Will wait for direction from @hsoh-u as to whether or not Atlas is also required.

@JohnHalleyGotway JohnHalleyGotway changed the title Enhance MET to compile and link against the Proj and Atlas libraries Enhance MET to compile and link against the Proj and eckit libraries (and maybe Atlas) Aug 23, 2023
@JohnHalleyGotway JohnHalleyGotway changed the title Enhance MET to compile and link against the Proj and eckit libraries (and maybe Atlas) Enhance MET to compile and link against the Proj and ecKit libraries (and maybe Atlas) Aug 23, 2023
@JohnHalleyGotway JohnHalleyGotway changed the title Enhance MET to compile and link against the Proj and ecKit libraries (and maybe Atlas) Enhance MET to compile and link against the ecKit and/or Atlas libraries Aug 23, 2023
@hsoh-u
Copy link
Collaborator

hsoh-u commented Aug 24, 2023

Please add atlas library and eckit library. To use KDTree at eckit, more work for point data (what atlas does) should be done.

@JohnHalleyGotway JohnHalleyGotway changed the title Enhance MET to compile and link against the ecKit and/or Atlas libraries Enhance MET to compile and link against the ecKit and Atlas libraries Aug 24, 2023
@JohnHalleyGotway
Copy link
Collaborator Author

Shifting from the beta1 cycle to beta2 since beta1 ends tomorrow. Would ideally like to complete this by the end of this week though.

@JohnHalleyGotway JohnHalleyGotway changed the title Enhance MET to compile and link against the ecKit and Atlas libraries Enhance MET to compile and link against the Atlas and ecKit libraries Sep 19, 2023
@JohnHalleyGotway
Copy link
Collaborator Author

@hsoh-u I was going to take a crack at this but am wondering how I can be most helpful to you. Should I use a separate feature_2574_atlas_eckit branch or should I use your existing development branch? Also, should I actually update any of the MET Makefiles to actually like to atlas and eckit? If so, which ones and where?

@hsoh-u
Copy link
Collaborator

hsoh-u commented Sep 20, 2023

Either ways work. The duplicated works were done at feature_2574_atlas_eckit branch (Makefile.am files and and others configurations). "development.seneca" maybe modified depending on the location of Atlas and Eckit locations.

@JohnHalleyGotway
Copy link
Collaborator Author

Per @TaraJensen on 9/21/23, make Atlas and ecKit OPTIONAL dependencies rather than required. There is potential that we might need add an ESMF-based solution in the future. And we don't want to paint ourselves into a corner.

@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Sep 27, 2023

During the METplus Governance Meeting on 9/27/23, @robdarvell from UK Met Office and @easatterfield from NRL both confirmed that making Atlas/ecKit optional rather than required is a good choice. This reinforces the decision last week from @TaraJensen.

@jprestop jprestop mentioned this issue Oct 6, 2023
15 tasks
@hsoh-u
Copy link
Collaborator

hsoh-u commented Oct 9, 2023

Which configuration option for ./configure?

  • --enable-ugrid
  • --enable-atlas
  • --enable-esmf
  • --enable-esmf-libs

This implies that the default is disabled

@jprestop
Copy link
Collaborator

jprestop commented Oct 9, 2023

Great question, @hsoh-u! I think my vote would be for "--enable-atlas", but I can see where one of the others would be preferred. I'm guessing that @TaraJensen and @JohnHalleyGotway would be better advisors than myself. I'm tagging them to help ensure they see your question.

@hsoh-u
Copy link
Collaborator

hsoh-u commented Oct 10, 2023

Thank you for the vote. Switching between --enable-atlas and --enable-ugrid is not difficult. I lean forward to --enable-ugrid. The following names will be used internally for the developers. But it may be exposed to MET users as the build documentation: WITH_UGRID or WITH_ATLAS at *cc, *h, and Makefiles, UGRID_LIBS or ATLAS_LIBS at Makefiles).

compile with/without -DWITH_ATLAS (generated by calling ./configure).
ATLAS_LIBS="-lvx_data2d_ugrid -latlas -leckit -leckit_mpi -leckit_geometry 

hsoh-u pushed a commit that referenced this issue Oct 10, 2023
hsoh-u pushed a commit that referenced this issue Oct 10, 2023
hsoh-u pushed a commit that referenced this issue Oct 10, 2023
hsoh-u pushed a commit that referenced this issue Oct 10, 2023
@JohnHalleyGotway
Copy link
Collaborator Author

Adding @jprestop as an assignee since she'll work on updates to the compile_MET_all.sh script for Atlas/ecKit.

@hsoh-u
Copy link
Collaborator

hsoh-u commented Oct 18, 2023

From Downloading and building Atlas

[ ecbuild ]

export ecbuild_ROOT=/d1/personal/hsoh/third_party/install/ecbuild
git clone https://github.com/ecmwf/ecbuild
cd ecbuild
mkdir build && cd build
cmake ../ -DCMAKE_INSTALL_PREFIX=$ecbuild_ROOT
make install

[ ecKit ]
Downloading and building - eckit
Prepare eckit source code:

wget https://github.com/ecmwf/eckit/archive/refs/tags/1.20.2.tar.gz
mv 1.20.2.tar.gz eckit-1.20.2.tar.gz
tar xvzf eckit-1.20.2.tar.gz
cd  eckit-1.20.2

Building eckit at seneca:

export PATH=$ecbuild_ROOT:$PATH
export eckit_ROOT=/d1/personal/hsoh/third_party/install/eckit
mkdir build && cd build
cmake ../ -DCMAKE_INSTALL_PREFIX=$eckit_ROOT
make install

Note: MPI is not enabled (add -DENABLE_MPI=ON to the cmake configuration line to enable MPI)

[ Atlas ]
Prepare atlas source code:

wget https://github.com/ecmwf/atlas/archive/refs/tags/0.30.0.tar.gz
mv 0.30.0.tar.gz atlas-0.30.0.tar.gz
tar xvzf atlas-0.30.0.tar.gz
cd atlas-0.30.0

Building Atlas at seneca:

export PATH=$eckit_ROOT/bin:$PATH
export atlas_ROOT=/d1/personal/hsoh/third_party/install/atlas
mkdir build && cd build
cmake ../ -DCMAKE_INSTALL_PREFIX=$atlas_ROOT
make -j8
make install

@hsoh-u hsoh-u closed this as completed Oct 18, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in MET-12.0.0 Development Oct 18, 2023
@jprestop
Copy link
Collaborator

Reopening so that I can add the changes to the compile_MET_all.sh script.

@JohnHalleyGotway
Copy link
Collaborator Author

@jprestop is working on this on derecho. Still planning on doing this during the beta2 dev cycle by Nov 7th.

JohnHalleyGotway added a commit that referenced this issue Nov 2, 2023
…ince its declared as const. The GNU compiler on seneca errors out when constants are included in the shared list while the GNU compiler on my Mac laptop doesn't complain.
@jprestop
Copy link
Collaborator

jprestop commented Nov 2, 2023

@hsoh-u I'm starting to update the compile_MET_all.sh script for ecKit and Atlas. I was looking in configure.ac in your branch to ensure that the environment variables were named MET_ECKIT and MET_ATLAS to specify the paths to the lib and include files and was checking for the -enable-ugrid option, but I don't see any of those in configure.ac. Do you have changes you haven't pushed yet or is that part of the work not completed yet?

@hsoh-u
Copy link
Collaborator

hsoh-u commented Nov 2, 2023

I built it yesterday from the feature branch (seneca:/d1/personal/hsoh/git/pull_request/MET_feature_2231_unstructured_grid).
MET_ATLAS
MET_ACKIT
--enable-ugrid

@jprestop
Copy link
Collaborator

jprestop commented Nov 2, 2023

Thank you, @hsoh-u. It's good to know that I was looking in the wrong branch! I appreciate the help and the confirmation of those variables and option.

@hsoh-u
Copy link
Collaborator

hsoh-u commented Nov 2, 2023

Sorry, it's not merged into the develop branch yet (it's in review status)

@jprestop
Copy link
Collaborator

jprestop commented Nov 2, 2023

@hsoh-u No need to be sorry at all. Being in a feature branch is totally fine. I just needed to know where to find this information to confirm. :)

JohnHalleyGotway added a commit that referenced this issue Nov 3, 2023
…ld rather than the individual object fields. And compute pair intersection, union, and sym diff counts from the split field rather than the object fields. This avoid allocating memory for many, many copies of the input grid, which can be quite large depending on the resolution.
@jprestop
Copy link
Collaborator

jprestop commented Nov 6, 2023

I have added eckit and atlas to the compilation script. I am testing on Derecho, but encountering problems with atlas being able to find ecbuild. I will be working on that today.

@jprestop
Copy link
Collaborator

jprestop commented Nov 6, 2023

That was an easy fix. On to the next problem which should also be an easy fix. Then I will check in the changes to my branch and will start updating the METbaseimage.

@jprestop
Copy link
Collaborator

jprestop commented Nov 6, 2023

Hi @hsoh-u. I checked out your branch, feature_2231_unstructured_grid, and ran bootstrap. I created a tar file to test with the changes to the compilation script. MET got through the "configure" step, the "make" step, the "make install" step, but failed on the "make test" step. I'm getting the following output in the make_test.log file:

make[1]: *** No rule to make target 'ugrid', needed by 'all'.  Stop.
make[1]: *** Waiting for unfinished jobs....

I have attached the met.make_test.log file here:
met.make_test.log

I'm not quite sure how to resolve this problem.

@hsoh-u
Copy link
Collaborator

hsoh-u commented Nov 6, 2023

The ugrid target is disabled for the "test" task.

@jprestop
Copy link
Collaborator

jprestop commented Nov 7, 2023

I am running ./build_docker_image.sh to test my changes in METbaseimage. It's taking a long time, but has been running well so far.

@jprestop jprestop linked a pull request Nov 16, 2023 that will close this issue
15 tasks
@jprestop
Copy link
Collaborator

This work was completed with PR #2737.

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in MET-12.0.0 Development Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: build process Build process issue priority: blocker Blocker requestor: METplus Team METplus Development Team type: new feature Make it do something new
Projects
Status: 🏁 Done
Development

Successfully merging a pull request may close this issue.

3 participants