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

Graphs #22

Closed
fsktom opened this issue Mar 29, 2023 · 16 comments · Fixed by #24
Closed

Graphs #22

fsktom opened this issue Mar 29, 2023 · 16 comments · Fixed by #24
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@fsktom
Copy link
Owner

fsktom commented Mar 29, 2023

I've been procrastinating on this for so long. It's possibly the most important aspect of this program. It's the reason I created this - Python was so slow for creating relative (% of plays at each time) graphs...

It's been 11 months since I started this Rust project, time to include graphs :)

@fsktom fsktom added enhancement New feature or request question Further information is requested labels Mar 29, 2023
@fsktom fsktom self-assigned this Mar 29, 2023
@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

- plotting
- either "plotpy - crates.io: Rust Package Registry" <https://crates.io/crates/plotpy>
- or automatically call a Python script on your own <https://www.reddit.com/r/rust/comments/8h22h3/graphing_in_rust/dygs2xt?utm_medium=android_app&utm_source=share&context=3>
- see [this repo](https://github.com/bheisler/cargo-criterion) for `gnuplot` and `plotters` usage

- [Plotters](https://old.reddit.com/r/rust/comments/ude3lz/plotters_is_back/) for graphs?

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

the Python version uses matplotlib https://github.com/fsktom/endsong-parser-python

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

probably plotly.rs
https://lib.rs/crates/plotly

I like that plot.show(); would open the default web browser (open HTML file). I could create HTML files for every graph created.
e.g.
folder plots/
with files <info_about_plot> - <time>.html hmm

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

even if plotters is used by FAAAR more crates and seems more flexible

but probably more difficult to get in to

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

Use multiple plotly traces for a compare command? (e.g. compare two artists)

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

I love the sensible defaults in plotly. I don't have to manually scald the graph or set the dimensions <3

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

W T F
ChatGPT reduced my code that runs (in release mode) for about 30s to about 5s wtffff
this function is called very often

fn gather_artist_date(
    entries: &Vec<SongEntry>,
    art: &Artist,
    start: &DateTime<Tz>,
    end: &DateTime<Tz>,
) -> u32 {
    let mut plays = 0;

    for entry in entries {
        let artist = Artist::from_str(&entry.artist);

        if artist == *art && entry.timestamp.ge(start) && entry.timestamp.le(end) {
            plays += 1;
        }
    }

    plays
}

ChatGPT recommendation with some changes of mine ->

fn gather_artist_date(
    entries: &[SongEntry],
    art: &Artist,
    start: &DateTime<Tz>,
    end: &DateTime<Tz>,
) -> usize {
    entries
        .iter()
        .filter(|entry| {
            entry.artist.eq(&art.name) && entry.timestamp.ge(start) && entry.timestamp.le(end)
        })
        .count()
}

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

Ok, it seems like constantly creating a new Artist was the reason for the low performance and the functional-style iterator thingy did not improve performance (very likely due to compiler optimizations with --release), but comparing Strings directly without reallocating them.

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

The problem will be then more clutter with comparing albums and esp. songs

fsktom added a commit that referenced this issue Mar 29, 2023
See #22
<#22 (comment)>

It looks worse and I think I've done this the first time, then went
to creating aspects, dervied Eq for them, and compared those directly
for it to look nicer. But seems like allocating a new aspect every time
is very time-consuming. Comparing Strings directly without re-allocating
anything is the way to go apparently.

For normal single usage it doesn't matter. But I discovered this while
working on plots and there I have to call such a function for every data
point (bc I don't want to lower resolution it's equivalent to every time
the aspect exists in the dataset heh). There it makes the code 6x faster
@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

I'm so happy that I got it working 😄

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

plot.show() doesn't work in Windows (also WSL) :))))
plotly/plotly.rs#132

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

The problem will be then more clutter with comparing albums and esp. songs

such an easy fix....
I already had the Music trait.... just add a function to compare to a SongEntry :)
done in 5cb091d

@fsktom
Copy link
Owner Author

fsktom commented Mar 29, 2023

ehm... on my MacBook... it's basically instant - less than a second I think
doing something like (in release mode)

>>> gartr
Artist name?
  >> Freedom cALL

w t f

@fsktom
Copy link
Owner Author

fsktom commented Mar 30, 2023

Why is WSL performance sooo bad. And the input delay while typing...

@fsktom fsktom mentioned this issue Mar 30, 2023
@fsktom fsktom linked a pull request Mar 30, 2023 that will close this issue
@fsktom
Copy link
Owner Author

fsktom commented Mar 30, 2023

See #23 for plot comparison

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant