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

Creation of services in OSCAR #7

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

vicente87
Copy link

  • The codes of the services are modified to adapt them to the asynchronous invocations of OSCAR.
  • A new service (EMG) is created that is used to join the N embedding and pass them as input parameters to the DDS.
  • Several elements are created in the Makefile to create the images of the services.
  • The creation_service folder has the FDL files to create the services in the different OSCAR clusters

Copy link
Collaborator

@jaime-cespedes-sisniega jaime-cespedes-sisniega left a comment

Choose a reason for hiding this comment

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

@vicente87 Please review the pending comments.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 This file is empty.

Copy link
Author

@vicente87 vicente87 Jul 14, 2023

Choose a reason for hiding this comment

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

@jaime-cespedes-sisniega You are right, this file does not have to be there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 This file is empty.

Copy link
Author

@vicente87 vicente87 Jul 14, 2023

Choose a reason for hiding this comment

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

@jaime-cespedes-sisniega You are right, this file does not have to be there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 This file is empty.

Copy link
Author

@vicente87 vicente87 Jul 14, 2023

Choose a reason for hiding this comment

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

@jaime-cespedes-sisniega You are right, this file does not have to be there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 This file is empty.

Copy link
Author

@vicente87 vicente87 Jul 14, 2023

Choose a reason for hiding this comment

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

@jaime-cespedes-sisniega You are right, this file does not have to be there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 This file must not be modified. It is generated by the training.py script.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 Probably most of these packages are not required for this service.

Copy link
Author

@vicente87 vicente87 Jul 14, 2023

Choose a reason for hiding this comment

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

@jaime-cespedes-sisniega Here you are absolutely right. What happened was that the service creation pattern that you provided us was used and when we created the EMG service, we forgot to remove the libs that were not going to be used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 This file must not be modified. It is generated by the training.py script.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 Why do you create this main-service.py when there is a main.py?

Copy link
Author

@vicente87 vicente87 Jul 14, 2023

Choose a reason for hiding this comment

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

@jaime-cespedes-sisniega The main-service.py is the modification of the main.py created by you, where the functions created have been reused to accommodate them to the invocation of MinIO services. Here output files are created so that they can be used by subsequent services and then conform to the functions created by you. But basically what the main-service.py (of the MLIS, DRS and DDS services) have is to accommodate the inputs and outputs of the files left in the MinIO buckets to be used by the previously created functions. Which perhaps could have been called main.py to follow the same structure that you created.

@@ -28,7 +29,9 @@ build-push-dimensionality-reduction:
docker buildx build --platform linux/amd64,linux/arm64 -t ifcacomputing/dimensionality-reduction-api --push dimensionality_reduction_api

build-push-model-inference:
docker buildx build --platform linux/amd64,linux/arm64 -t ifcacomputing/model-inference-api:latest --push model_inference_api
docker build -t ghcr.io/grycap/mls-arm-api model_inference_api
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 Why do you change the registry where this image is stored?

docker buildx build --platform linux/amd64,linux/arm64 -t ghcr.io/grycap/dds-arm-api --push detector_api

emc:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vicente87 I think that this should be the unique "new" service. The addition of the others (mls, dds and drs) is completely redundant. Because they are already hosted in ifcacomputing/XXXX-api.

Copy link
Author

@vicente87 vicente87 Jul 14, 2023

Choose a reason for hiding this comment

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

@jaime-cespedes-sisniega The problem is that in order to make the asynchronous invocations we create main-service.py (it's from what was put in main.py) and there we introduce small changes to be able to orchestrate the services well among others. For example we create output files in the drs services in json format so that you can use it well. Also, the input data that comes from the MinIO buckets has to accommodate the functions that you have created and that is why we decided to create the new images. I don't know if I will explain well.

Copy link
Collaborator

@jaime-cespedes-sisniega jaime-cespedes-sisniega left a comment

Choose a reason for hiding this comment

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

@vicente87 Since there is a conflict with some objects that are generated with the ml/mnist/training.py script, I suggest deleting the ones you intend to upload, since the latest version of them is already in the repo and these that are intended to be uploaded are already old versions. Thank you.

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.

2 participants