Model card design

[BenjaminBossan]

I would like to suggest that we go back to the drawing board to discuss the model cards feature. Below you can find my opinions but of course feel free to disagree.

User

What does the user of this feature want to achieve?

As a user...

1. I want to have a convenient way of creating a model card of my trained sklearn model (or sklearn-compatible model).
2. I don't want to make big changes to my existing code to be able to use model cards.
3. I don't want to be required to learn new concepts for creating a model card.
4. I want to output different model card formats (perhaps?).
5. I want to have arbitrary sections/figures/tables etc. in the model card (perhaps?).

Supported metrics

Ideally, we want to be able to use as much as possible of what sklearn already provides. These things come to my mind:

1. Scalar metrics (accuracy, f1, r2, etc.). Do we want to support used-defined scalar metrics?
2. Confusion matrix.
3. CV results (hyper param search, cross_validate)
4. classification_report
5. Some kind of feature importance.
6. Model visualizations?

Note that it's absolutely possible to start with a subset of those features and add more later on.

API

Here I believe adoption will be better if users can use their existing code bases/scripts and only need to add a few lines to be able to create model cards.

Below are some suggestions I collected:

clf = SVC(random_state=0)
clf.fit(X_train, y_train)

predictions = clf.predict(X_test)
cm = confusion_matrix(y_test, predictions, labels=clf.classes_)
disp = ConfusionMatrixDisplay(confusion_matrix=cm,
                              display_labels=clf.classes_)
card.add_inspection('Confusion Matrix', disp)

model.fit(X, y)
y_pred = model.predict(X_test)
accuracy = accuracy_score(y_test, y_pred)
clf_report = classification_report(y_test, y_pred)
conf_matrix = confusion_matrix(y_test, y_pred)

# everything above is just vanilla sklearn, below comes model card stuff
model_card = skops.card.create_model_card(accuracy=accuracy, clf_report=clf_report, conf_matrix=conf_matrix, description=...)
model_card.save(...)

Implementation

It could be advantageous if we use a more generic representation of a "card". This will allow us to more easily change:

• the card format (if there is no fixed specification)
• the storage back end (disk, remote)
• formats (markdown, rst, json/yaml?)

On the other hand, we could make things easier by directly relying on Nate's model card library, making this feature a thin wrapper around it. This will result in less work here but tightly couples this feature to that specific implementation.

[merveenoyan]

[Copied and pasted from the other issue I wrote about injecting slots after card creation]

Opening discussion for #26 (review) by @adrinjalali.

As with the current implementation in modelcards library, it’s not really possible to inject values to jinja template after the card creation (I also talked to @nateraw and he told me it’s made to be this way).

I looked at jinja’s own docs and saw the rendering takes place once with a dictionary you pass in (it takes place when we initialize the RepoCard) so below is how modelcards library uses render as well like below:

content = jinja2.Template(Path(template_path).read_text()).render(
            card_data=card_data.to_yaml(), **template_kwargs
        )

so technically you can’t inject anything after you initialize the model card. as of currently implementation we have in skops, it’s not really possible to inject things after creation as well.

So I thought of having a separate card class made for skops and separate initialization and letting user pass attributes and at the end, get these attributes and pass as a dict to RepoCard template when the user calls save().
so the UX is a bit like this (which doesn't change anything except for adding variables during creation)

#below card class is our card class
model_card = card.create_model_card(
    model,
    card_data=card_data,
    template_path=“./skops/skops/card/default_template.md”,
    limitations=limitations,
    model_description=model_description,
)
card.metrics = some_metrics
card.save(path) # this method will initialize `RepoCard` class of modelcards, pass the class attributes as dictionary to that, renders (which is done in `modelcards`), and save the card

Maybe we can let user add things as they wish to the template, meaning they can add as many plots as they wish by simply editing the markdown, like below:

card.roc_plot = roc_plot # this normally doesn't exist in template, we'll add it at the end with attribute setting
card.save(path)

(above was my previous comment)
As told by @BenjaminBossan it might be better to let user not set things as they wish so having a method to do this might be better like what @adrinjalali suggests (see below)
card.add_inspection('Confusion Matrix', disp)

[adrinjalali]

I agree with all you say @BenjaminBossan , but here are some points:

I want to output different model card formats (perhaps?).

I personally agree with you, but for now we're focusing on what the huggingface hub requires and that's a README.md file which has some metadata on top of it, used by the hub's backend.

I want to have arbitrary sections/figures/tables etc. in the model card (perhaps?).

Yes, but we don't have to add this from the beginning I'd say. We can certainly add this as a future feature.

Scalar metrics (accuracy, f1, r2, etc.). Do we want to support used-defined scalar metrics?

If we accept a scorer, then we support user defined functions, and we should.

Confusion matrix.

Yes, but this is "classification" specific, other tasks might have very different types of metrics.

Some kind of feature importance.

More like inspections/interpretations which can go beyond importances of features, but yes.

It could be advantageous if we use a more generic representation of a "card". This will allow us to more easily change:

the card format (if there is no fixed specification)
the storage back end (disk, remote)
formats (markdown, rst, json/yaml?)

On the other hand, we could make things easier by directly relying on Nate's model card library, making this feature a thin wrapper around it. This will result in less work here but tightly couples this feature to that specific implementation.

At least to start with, the idea is to support the requirements of the HF hub (especially when it comes to metadata), and I rather not duplicate that part of the code. As for the format, we have decided to stick to README.md on the hub since the model is initially generated by programmers, but might be edited by other non-programmer people on sections where they can contribute (a product owner for instance). But we should always be able to generate a printable form, from the rendered version on the hub, or the md files.

I'm happy to support more diverse formats/outputs/backend storage in the future if there is need, but there are quite a few modelcard libraries and projects already out there that I'm not sure if we want to compete in that space.

As with the current implementation in modelcards library, it’s not really possible to inject values to jinja template after the card creation (I also talked to @nateraw and he told me it’s made to be this way).

Basically, since we want to allow users to edit the .md files from the frontend, if we store the modelcard in a different format in the users' repo, the two files: the README.md and the card.myfancyformat would go out of sync quite easily, unless we do significant work on the backend side to edit the card object instead of the md file. Therefore for now, we're only storing the README.md file, and that means we can't parse it back into a format which we can use for jinja stuff

so technically you can’t inject anything after you initialize the model card. as of currently implementation we have in skops, it’s not really possible to inject things after creation as well.

So I thought of having a separate card class made for skops and separate initialization and letting user pass attributes and at the end, get these attributes and pass as a dict to RepoCard template when the user calls save().
so the UX is a bit like this (which doesn't change anything except for adding variables during creation)

100% on the concept of an intermediate object type to store everything needed, but with the card.add_inspection API kinda thing.

Hope this makes sense, please let me know if there are parts of my mumbling incoherent lol

[merveenoyan]

@adrinjalali so I'm currently working on sort of an intermediate object that in the end you only get @nateraw's model card and store that only, but you will not be able to go back to that object after you pushed it to Hub or saved it. It's only for when you create the card, you can inject values right after that.

[adrinjalali]

That sounds good to me :)

[merveenoyan]

So I changed the UX a little like below:

#below card class is our card class
model_card = Card(
    model,
    card_data=card_data,
    limitations=limitations,
    model_description=model_description,
)
card.add_inspection(some_plot)
card.save(path)

or

We can also do:

model_card = Card(model) # where model card related stuff will be generated, like hyperparam table
card.add_inspection(some_plot) # add whatever you want
card.save(card_data, dict_of_jinja_slots, path) # add stuff like model description and save to modelcards and README.md

WDYT?

class Card:

    def __init__():
        pass

    def add_inspection():
        pass

    def save():
        # this turns card into modelcards format, renders jinja template and saves
        pass

Anything else you want to add? @adrinjalali @BenjaminBossan

[adrinjalali]

A third option is to have whatever's accepted in jinja template as an exposed attribute, or allow a set method to set them, like:

model_card = Card(model='model.pkl').set(
    limitations="my limitation text, authors='me'
).set(references='my references')

model.save(path)

set and add_inspection would both return self for them to be chained by the user.

[merveenoyan]

Above we talked about not letting user set things, are we sure?

[BenjaminBossan]

I think there is no reason not to have a fluent interface. But maybe set is not a good name because if the builtin set? How about add?

Above we talked about not letting user set things, are we sure?

Did we agree on that? :D What is the disadvantage?

[adrinjalali]

I must have missed it then, where did we decide not to set things?

[merveenoyan]

(follow-up from Slack) So what saving does is that it’s where the modelcards format is initialized and jinja template is rendered and saved to a target repository, thus we can actually pass the things set by user (like metadata or sections of model card).
other option is to pass them during initialization of the intermediate class (Card) that I’m writing.
so in one, sections of jinja template/MD file are passed during saving and in another, sections are passed during initialization.

[BenjaminBossan]

So what saving does is that it’s where the modelcards format is initialized and jinja template is rendered and saved to a target repository

👍 from me. That way, the user doesn't need to bother with modelcards or even need to know about its existence. Furthermore, it makes it easier to later support other model card frameworks if we want to.

[merveenoyan]

I remember somewhere that we decided not to define setter methods that user would interact. We can surely do that, but +1 on @BenjaminBossan's comment on not to call them set.

[BenjaminBossan]

If we want to support arbitrary metrics, it's almost inevitable to provide methods to add them. One possible issue with Adrin's suggestion would be if we need different methods for different types of data to be added, e.g. a scalar metric vs a plot vs a table vs text. I'd rather have the user call specific methods than us trying to guess the intent from the input type.