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

Granular compiler tests #589

Merged
merged 9 commits into from
Nov 8, 2023
Merged

Granular compiler tests #589

merged 9 commits into from
Nov 8, 2023

Conversation

PhilippeMoussalli
Copy link
Contributor

Addresses https://github.com/ml6team/fondant-use-cases/issues/46

For each compiler we now have a method get_pipeline_configs() to fetch the essential information about a pipeline (component, arguments, dependencies, hardware, ...) from a compiled specification file. In this way, we no longer face issues with having to modify the specification files after every small modification to the framework. It also makes it easier to call the desired attribute without having to dive deep into the schema of the compiler.

@GeorgesLorre
Copy link
Collaborator

Nice changes, will definitely make testing more easy.

I do wonder if we can't do more with these ComponentConfigs ? Now they are a bit of an afterthought and only used for testing. Maybe we can also use them to configure the components for a runner ?

@PhilippeMoussalli
Copy link
Contributor Author

Nice changes, will definitely make testing more easy.

I do wonder if we can't do more with these ComponentConfigs ? Now they are a bit of an afterthought and only used for testing. Maybe we can also use them to configure the components for a runner ?

Thinking about it now, they seem to very similar to the componentOp main difference is that the componentOp defines static configs whereas the ComponentConifgs include some attributes similar to the ComponentOp with some additional dynamic configs that can only be inferred at compile time (context, volumes, context, ...).
How would you want to use them for a runner?

@RobbeSneyders
Copy link
Member

Not sure yet how to use them, but if we only use them for testing, I would remove them from the core Fondant code. We can either

  • Move them to our test files
  • Create a testing.py module if we think it can help users test Fondant pipelines as well. Connexion example

@GeorgesLorre
Copy link
Collaborator

GeorgesLorre commented Nov 6, 2023

I think we are all feeling the same thing: This is really clean but we only use it for testing right now so either:

  • use it for more (like configuring a compiler/runner)
  • or make it test specific only (and then maybe indeed move it from the core)

@PhilippeMoussalli
Copy link
Contributor Author

Thanks for the suggestions, moved them over to a separate testing.py

@PhilippeMoussalli PhilippeMoussalli merged commit c9f84a8 into main Nov 8, 2023
6 checks passed
@PhilippeMoussalli PhilippeMoussalli deleted the granular-compiler-tests branch November 8, 2023 13:20
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.

3 participants