Skip to content

docs: add target modules documentation#331

Merged
gsprochette merged 6 commits intomainfrom
docs/add-target-modules-documentation
Sep 17, 2025
Merged

docs: add target modules documentation#331
gsprochette merged 6 commits intomainfrom
docs/add-target-modules-documentation

Conversation

@gsprochette
Copy link
Copy Markdown
Collaborator

Description

Add documentation for target modules:

  • a documentation page target_modules.rst, referenced as a drop-down from SmashConfig
  • change printed hyperparameters so that TargetModules are correctly shown on the algorithm page
  • provide a documentation_name_with_link attribute in target modules to easily link to the doc in algorithms
  • add a target modules tutorial with demonstration on quanto

Related Issue

Fixes #(issue number)

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Building the documentation

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Additional Notes

I think an other documentation page about how to use the map_targeted_nn_roots function would have its place somewhere in the "Customize components" section, but this is left for an other PR

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Copy Markdown
Contributor

@minettekaum minettekaum left a comment

Choose a reason for hiding this comment

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

Found some small fixes in the notebook but otherwise looks good :)

Copy link
Copy Markdown
Collaborator

@sdiazlor sdiazlor Sep 10, 2025

Choose a reason for hiding this comment

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

"from Quanto:"


Reply via ReviewNB

Copy link
Copy Markdown
Collaborator

@sdiazlor sdiazlor Sep 10, 2025

Choose a reason for hiding this comment

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

5.Compare the results


Reply via ReviewNB

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I also went through all 1-5 titles and made every noun start with a capital letter, previously that was inconsistent

Copy link
Copy Markdown
Collaborator

@sdiazlor sdiazlor left a comment

Choose a reason for hiding this comment

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

LGTM! I added just some tiny comments to improve the readability :)

@gsprochette gsprochette force-pushed the docs/add-target-modules-documentation branch 3 times, most recently from 65f4cb6 to 1e3e557 Compare September 10, 2025 14:57
Copy link
Copy Markdown
Member

@sharpenb sharpenb left a comment

Choose a reason for hiding this comment

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

I left some quick comments. That is very nice. I kind of see the point of having cells outputs for tutorials since it amkes notebooks more attractive. As discussed, this can be addressed in another PR to introduce it on all tutorials :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we find a more catchy name here e.g. "Compressing at Finer Granularity with Target Modules"? :)

Copy link
Copy Markdown
Collaborator Author

@gsprochette gsprochette Sep 17, 2025

Choose a reason for hiding this comment

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

I went with "Smashing at Finer Granularity with Target Modules", is that ok for you?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See the other comment to use a more catchy name ;) This is the technical name byt the tile should explain the value for the users :)

Copy link
Copy Markdown
Collaborator Author

@gsprochette gsprochette Sep 17, 2025

Choose a reason for hiding this comment

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

I went with "Selective Smashing with Target Modules"

@gsprochette gsprochette force-pushed the docs/add-target-modules-documentation branch from 8315063 to 52b6dab Compare September 17, 2025 13:09
@gsprochette gsprochette force-pushed the docs/add-target-modules-documentation branch from 52b6dab to 0c368b4 Compare September 17, 2025 13:36
@gsprochette gsprochette merged commit 91bf167 into main Sep 17, 2025
7 checks passed
@gsprochette gsprochette deleted the docs/add-target-modules-documentation branch September 17, 2025 14:21
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.

4 participants