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

Fix #1332: Add benchmark plots #1613

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

graceful-coder
Copy link
Collaborator

Fixes #1332

@graceful-coder graceful-coder self-assigned this Sep 17, 2024

def plot_data(filename, calibration, autoregressive_n, y_column):
"""
Plots the data from the given CSV file.
Copy link
Contributor

Choose a reason for hiding this comment

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

a clear alternative is below

def _create_plot(df, calibration, autoregressive_n, y_column, title):
    traces = generate_traces(df, calibration, autoregressive_n, y_column)
    yaxis_title = (
        "Predictoor Profit (OCEAN)"
        if y_column == "pdr_profit_OCEAN"
        else "Trader Profit (USD)"
    )
    fig = go.Figure(data=traces, layout=layout)
    fig.update_layout(
        title=f"{df['Model'].iloc[0]} - {title} - {y_column}",
        yaxis_title=yaxis_title,
    )
    fig.show()


def plot_data(filename, calibration, autoregressive_n, y_column):
    # Load data from CSV
    df_without_eth, df_with_eth = load_data_from_csv(filename)
    
    # Plot without ETH
    title_without_eth = "Predictoor Profit Benchmarks (Trained with BTC-USDT Data)"
    _create_plot(df_without_eth, calibration, autoregressive_n, y_column, title_without_eth)
    
    # Plot with ETH
    title_with_eth = "Predictoor Profit Benchmarks (Trained with BTC-USDT & ETH-USDT Data)"
    _create_plot(df_with_eth, calibration, autoregressive_n, y_column, title_with_eth)
    
    ```



def generate_traces(df, green_shades, y_column):
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks difficult to read. You might want to consider splitting this function.

)
temp_traces.append(trace)

traces.extend(reversed(temp_traces))
Copy link
Contributor

Choose a reason for hiding this comment

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

In this function, you append everythink to the traces and temp_traces variables are created inside, you append everything into the temp_traces (good), then you extend the traces variable by reversing the the temp_traces but the traces variable is already empty.

Why don't we do like that?

I assumed we have _get_group_data and _create_trace functions, think it like a pseudo-code

   traces = []

    # Grouping the dataframe
    grouped = df.groupby(["Model", "Calibration", "predictoor_ss.aimodel_data_ss.autoregressive_n"])

    # Sorting the groups by the maximum y_column value in descending order
    sorted_groups = (
        grouped[y_column].max().reset_index().sort_values(by=y_column, ascending=False)
    )

    # Generating the traces
    for _, row in sorted_groups.iterrows():
        group_df = _get_group_data(grouped, row)
        color = green_shades.pop(0)  # Get the next available color
        trace = _create_trace(group_df, row, color, y_column)
        traces.append(trace)

    return list(reversed(traces))
    ```

Copy link
Contributor

@kdetry kdetry left a comment

Choose a reason for hiding this comment

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

I am not familiar with the context of this task, but I wanted to make some suggestions about the code.

My codes that I shared could have some syntax errors

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.

[BMs] Make it easy to reproduce multisim benchmarking plots
2 participants