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

[Task]: Add logger to write to csv files. #2055

Open
ashwinvaidya17 opened this issue May 14, 2024 · 13 comments · May be fixed by #2226
Open

[Task]: Add logger to write to csv files. #2055

ashwinvaidya17 opened this issue May 14, 2024 · 13 comments · May be fixed by #2226
Assignees
Labels
Good First Issue Issues that can be picked up by someone unfamiliar with the repo and would like to contribute.

Comments

@ashwinvaidya17
Copy link
Collaborator

What is the motivation for this task?

It is handy to collect logged metrics to a csv file.

Describe the solution you'd like

  1. Add another logger in src/anomalib/loggers that writes metrics to a csv file.
  2. The solution will probably involve sub-classing Lightning's CSV Logger.
  3. Another good feature to add would be to print the collected metrics as a single table like the _print_tabular_results method in the benchmark job.

Additional context

No response

@ashwinvaidya17 ashwinvaidya17 added the Good First Issue Issues that can be picked up by someone unfamiliar with the repo and would like to contribute. label May 14, 2024
@zhaozzt
Copy link

zhaozzt commented Jun 11, 2024

Please teach me how to modify the code

@ChaitanyaYeole02
Copy link

Hey @ashwinvaidya17, Is this issue been worked on?
I am interested to work on this, can you please assign it to me?

@samet-akcay
Copy link
Contributor

Hi @ChaitanyaYeole02, no one is working on this at the moment. Thanks for your interest

@ChaitanyaYeole02
Copy link

Hey @samet-akcay, what kind of metric could be written in this CSV file?
As I saw for the other logging files, images are been logged.
Currently, I have written the basic class to log the metrics.

class AnomalibCSVLogger(CSVLogger):
    def __init__(
            self,
            save_dir: str,
            name: str | None = "default",
            version: int | str | None = None,
            prefix: str = "",
            flush_logs_every_n_step: int | None = 100,
    ) -> None:
        super().__init__(
            save_dir=save_dir,
            name=name,
            version=version,
            prefix=prefix,
            flush_logs_every_n_steps=flush_logs_every_n_step,
        )

@ChaitanyaYeole02
Copy link

Hey @samet-akcay, any suggestions?

@ashwinvaidya17
Copy link
Collaborator Author

@ChaitanyaYeole02 I tried your code locally. This is what it writes

afbeelding

Looks fine to me. Feel free to create a PR.

MRE

from anomalib.data import MVTec
from anomalib.engine import Engine
from anomalib.loggers.csv import CSVLogger
from anomalib.models import Padim


def main():
    logger = CSVLogger(save_dir="csv_test")
    engine = Engine(logger=logger)
    model = Padim()
    datamodule = MVTec()

    engine.train(datamodule=datamodule, model=model)


if __name__ == "__main__":
    main()

@ChaitanyaYeole02
Copy link

@ashwinvaidya17 I will raise the PR, but could you also tell me how can I setup and test this locally?

@ashwinvaidya17
Copy link
Collaborator Author

Currently, the subclassed logger provides the same features as the base logger. Do you plan to save separate test and train metrics? Epoch 0 and 1 will be a bit confusing. I tested this on Padim so ideally, we should get train metrics, validation metrics, and test metrics separately rather than showing up as epoch. Otherwise this does not provide any additional features on top of the base class.

@ChaitanyaYeole02 ChaitanyaYeole02 linked a pull request Jul 31, 2024 that will close this issue
9 tasks
@ChaitanyaYeole02
Copy link

ChaitanyaYeole02 commented Jul 31, 2024

Yeah, currently it does not provide any feature on top of the base class. Do you have any suggestion of additional features on top of the base class?

I was able to replicate the MRE. Should we add more metrics?

@ashwinvaidya17
Copy link
Collaborator Author

@ChaitanyaYeole02 thanks for creating the PR, but apologies, I realized that my issue is ill-defined. The original use-case came from the benchmarking script as it prints all the models, dataset categories, and metrics in a single table. Something like this

afbeelding

I was hoping we could get something similar for general training but looks like the information is not sufficient. With a closer look at the base class, it seems like we can only access the step along with the metrics dict. This makes it harder to identify which stage of training it is printed from. Ideally, it would be nice to save the metrics like

stage model dataset category image_F1 pixel_F1
train Padim MVTec bottle xx xx
test Padim MVTec bottle xx xx

But looks like it is not as straightforward as I thought.

@ChaitanyaYeole02
Copy link

Ohh, interesting. I would take a look at this. How did you get these benchmark results any MRE?

@ashwinvaidya17
Copy link
Collaborator Author

@ChaitanyaYeole02
Copy link

Thank you, let me take a look at this and if I am stuck somewhere I will let you know.

samet-akcay added a commit to ChaitanyaYeole02/anomalib that referenced this issue Aug 16, 2024
@samet-akcay samet-akcay linked a pull request Aug 19, 2024 that will close this issue
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Issues that can be picked up by someone unfamiliar with the repo and would like to contribute.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants