Add ability to automatically generate Kitfiles by amisevsk · Pull Request #661 · kitops-ml/kitops

amisevsk · 2024-12-12T21:40:11Z

Description

Add kit init command, that can be used to generate a basic Kitfile from the contents of a directory. It basically does this by looking at files and trying to guess what they might be, based on file extensions.

I've tested it against a few hand-written Kitfiles and (modulo user-provided metadata) it does an okay job :)

Linked issues

Closes #124
Related: #564

gorkem · 2024-12-16T16:00:44Z

+	if err != nil {
+		return "", fmt.Errorf("failed to read input: %w", err)
+	}
+	return strings.TrimSpace(string(bytes)), nil


Should this be trimming? What if the prompt is intended to have spaces?

strings.TrimSpace only trims leading and trailing whitespace.

right... but I was thinking is there a case where we want to keep the leading/trailing spaces

I can't imagine one, and not trimming likely leads to weirder behaviour than otherwise. This code was originally used for handling login (username and password) where we definitely want to trim white-space, since the read input includes the newline delimiter.

gorkem · 2024-12-16T23:26:02Z

+	opts.configHome = configHome
+	opts.path = args[0]
+
+	if opts.modelkitName == "" {


Should we still prompt for if name or description has been passed as options?

We'll only prompt if we didn't get a name/description (and if the session is interactive). If you specify --name or --desc we don't prompt (unless you somehow pass in an empty name/description).

gorkem · 2024-12-16T23:28:18Z

+// Generate a basic Kitfile by looking at the contents of a directory. Parameter
+// packageOpt can be used to define metadata for the Kitfile (i.e. the package
+// section), which is left empty if the parameter is nil.
+func GenerateKitfile(baseDir string, packageOpt *artifact.Package) (*artifact.KitFile, error) {


I think this needs to look into the sub directories as well. If somebody organizes their files under data/ for instance data/training.csv it should be detected.

Yeah, I was hoping to defer that to a future improvement since it introduces a fair bit of complexity in the general case.

If we're trying to grab specific files (e.g. a dataset layer for path: data/training.csv, then we need to decide what to do with the rest of the directory

If that's the only file, we don't need to include the directory itself, since it'll be included in the dataset layer, which means signalling another condition back to the caller

If there are other files, we need to either classify them or include the whole directory as code (?) layer

Then we also need to handle the case where we've got a catch-all code section

If we want to include the whole directory (i.e. path: data/) then we need to figure out a heuristic for deciding what type the whole directory is (most common file type? I'm not sure here)

If a data/ dir has 20 .csv files, do we include a single data/ layer, or generate a Kitfile with 20 dataset layers (one for each file).

Alright, I've added a (messy) implementation of this in commit adf3372. For e.g. https://huggingface.co/onnx-community/dpt-dinov2-small-kitti/tree/main it generates the following Kitfile:

manifestVersion: 1.0.0 model: name: onnx/model path: onnx/model.onnx parts: - path: onnx/model_bnb4.onnx - path: onnx/model_fp16.onnx - path: onnx/model_int8.onnx - path: onnx/model_q4.onnx - path: onnx/model_q4f16.onnx - path: onnx/model_quantized.onnx - path: onnx/model_uint8.onnx - path: config.json - path: preprocessor_config.json - path: quantize_config.json code: - path: . docs: - path: README.md description: Readme file

where the code section includes the .gitattributes file and the .git directory (since I cloned it locally)

gorkem · 2024-12-16T23:32:32Z

It may be worth linking to this list here. https://github.com/trailofbits/ml-file-formats?tab=readme-ov-file

amisevsk · 2024-12-17T19:02:18Z

It may be worth linking to this list here. https://github.com/trailofbits/ml-file-formats?tab=readme-ov-file

I glanced over that list at one point, but I'm not sure how valuable it is in general. I can add it but it doesn't feel like an authoritative source for this sort of thing.

Try to attach the detected license type to a Kitfile layer rather than the overall package section -- i.e. if we have a model, assume the license applies to the model, otherwise, stick it with a dataset or code section. Fallback to package if nothing else makes sense.

…se.txt When deciding if a file is a license file, include all files with `license` as the prefix, to ensure we catch e.g. LICENSE.txt.

Add .joblib as model-type suffix, and move .csv to datasets instead of metadata

Read the contents of any directories and attempt to determine their type for the purposes of Kitfile generation; if a directory contains multiple files but e.g. they are all 'dataset'-type files, add the directory as a dataset layer. This is not a very pretty implementation, but hopefully covers a decent number of common cases.

gorkem

When I try this on

/scikitlearn-tabular
├── Kitfile
├── data
 │   └── online_boutique_v1.csv
 ── models
 │   └── model.onnx
└── notebooks
    └── ML_MLFlow.ipynb

I am getting the following. I was expecting the model.onnx to be added at least.

manifestVersion: 1.0.0
package:
  name: test
  description: this
code:
  - path: .

gorkem · 2024-12-19T21:25:37Z

+				output.Logf(output.LogLevelWarn, "Unable to determine license type")
+			}
+			output.Logf(output.LogLevelTrace, "Detected license %s for license file", detectedLicenseType)
+			detectedLicenseType = licenseType


This line should be before the logging.

Woops, good catch! Fixed.

amisevsk · 2024-12-19T21:58:31Z

@gorkem Hm, are you sure you've fetched the latest changes? I rebased on main as part of it so you would need to git reset --hard ... to see current state (or delete the branch locally and use gh pr checkout again).

I tested with

.
├── data
│   └── online_boutique_v1.csv
├── models
│   └── model.onnx
└── notebooks
    └── ML_MLFlow.ipynb

(files are just basic text files containing "test") and I see the Kitfile

manifestVersion: 1.0.0
model:
  name: model
  path: models/model.onnx
code:
  - path: notebooks
datasets:
  - path: data

amisevsk · 2024-12-19T23:01:00Z

Figured it out -- I was mixing up dirEntry.Name() and the full path to the directory. If you ran the init command from the directory you want to generate a Kitfile in it would work because it's looking for directories in the current working dir. This fails if kit is run from a different directory (I had tried to test this using kit init $(pwd) but that obviously misses this bug).

gorkem

Needs tests

When reading the contents of directories while generating Kitfiles, make sure we use the full path to open the directory.

amisevsk requested a review from gorkem December 12, 2024 21:40

amisevsk force-pushed the kitfile-generation branch from df6f11c to f2011c2 Compare December 12, 2024 21:42

gorkem mentioned this pull request Dec 14, 2024

init command to generate a kitfile #124

Closed

gorkem requested changes Dec 16, 2024

View reviewed changes

amisevsk force-pushed the kitfile-generation branch from f2011c2 to 51c8638 Compare December 17, 2024 21:51

amisevsk added 11 commits December 18, 2024 13:13

Add package for auto-generating Kitfiles from a directory

758b7dd

Don't print empty descriptions on docs layers

245580b

Add kit init command to CLI to generate Kitfile

11ab550

Add input for kit init command to provide name and description

efb84f3

Try to detect license when generating Kitfile

843ccf3

Fix handling of generated Kitfile paths when not current directory

8f4509a

Don't overwrite existing Kitfiles in kit init command; add --force flag

003a073

Relax check for licenses when generating Kitfiles to catch e.g. licen…

227c4c3

…se.txt When deciding if a file is a license file, include all files with `license` as the prefix, to ensure we catch e.g. LICENSE.txt.

Update file suffixes for Kitfile generation

8af06ff

Add .joblib as model-type suffix, and move .csv to datasets instead of metadata

amisevsk force-pushed the kitfile-generation branch from bacde8a to adf3372 Compare December 18, 2024 19:15

Skip existing Kitfiles when generating a new Kitfile.

0359443

amisevsk requested a review from gorkem December 18, 2024 19:45

gorkem reviewed Dec 19, 2024

View reviewed changes

Add trace logging to Kitfile generation

b279bc4

amisevsk force-pushed the kitfile-generation branch from 7fbd3d6 to 3dee98c Compare December 19, 2024 22:59

gorkem approved these changes Dec 20, 2024

View reviewed changes

kitfile-gen: Fix reading directories when not running in context dir

8879bfd

When reading the contents of directories while generating Kitfiles, make sure we use the full path to open the directory.

amisevsk force-pushed the kitfile-generation branch from 3dee98c to 8879bfd Compare December 20, 2024 16:44

amisevsk merged commit c381b40 into kitops-ml:main Jan 10, 2025

amisevsk deleted the kitfile-generation branch January 10, 2025 03:21

Conversation

amisevsk commented Dec 12, 2024

Description

Linked issues

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gorkem commented Dec 16, 2024

Uh oh!

amisevsk commented Dec 17, 2024

Uh oh!

gorkem left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amisevsk commented Dec 19, 2024

Uh oh!

amisevsk commented Dec 19, 2024

Uh oh!

gorkem left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants