NVIDIA / NVFlare

NVIDIA Federated Learning Application Runtime Environment
https://nvidia.github.io/NVFlare/
Apache License 2.0
596 stars 170 forks source link

Add hello examples with new APIs #2785

Closed nvkevlu closed 4 weeks ago

nvkevlu commented 1 month ago

Add hello examples with new APIs

Description

Add hello examples with new job and client APIs, re-based version of this: https://github.com/NVIDIA/NVFlare/pull/2723 but without deleting the previous hello examples for now. We need the current ones for CI, and we can still reorganize this.

Types of changes

chesterxgchen commented 1 month ago

can we fix the CI/CD with new APIs ? if we have both them in the hello example folder it defeat the original purpose.

YuanTingHsieh commented 1 month ago

Because these are new styles of example that currently are not supported in CI.

I could just move all old hello examples to testing directory and change CI to that dir for testing. So it does not confuses users. Users will only see new style of examples.

nvkevlu commented 1 month ago

Because these are new styles of example that currently are not supported in CI.

I could just move all old hello examples to testing directory and change CI to that dir for testing. So it does not confuses users. Users will only see new style of examples.

I think that sounds good. I was looking into the CI and are we actually testing from hello examples from the examples directory or just using the configurations under tests/integration_test/data?

nvkevlu commented 1 month ago

I decided to move and keep the existing CI jobs out of the way in test/integration_test/data/test_configs/standalone_job/previous_jobs, and for the new Job API jobs, I did not export jobs because I thought it would create too much extra clutter in the beginning that may confuse and intimidate new users, and if they are ready to look into the job, they can run the export job command themself when they are ready to look at it.

YuanTingHsieh commented 1 month ago

Great PR with a lot of details, added some comments

nvkevlu commented 4 weeks ago

CI doesn't pass because we are adding new pages, and the link checker does not consider pages being added as existing

YuanTingHsieh commented 4 weeks ago

Ok we can merge this then make sure ci passing after this merge

chesterxgchen commented 4 weeks ago

let's do that, we might have to consolidate hello-world with getting started and removed some duplication, we could have done that before the conversion :-(