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

Time series using "Values" has some quirks #61

Open
CodeByDrescher opened this issue Oct 15, 2024 · 10 comments
Open

Time series using "Values" has some quirks #61

CodeByDrescher opened this issue Oct 15, 2024 · 10 comments

Comments

@CodeByDrescher
Copy link

CodeByDrescher commented Oct 15, 2024

There seem to be two related bugs regarding time series requesting value output:

  1. When output start time is not defined by the user, using values will result in the first time value being overwritten to 0
    e.g.: values [10, 20, 30] results in a data frame with time [0, 20, 30]

  2. When output start time is defined by the user to work around this, the time start is non-inclusive.
    e.g. values [10, 20, 30, 40] with start at 10 gives outputs [20, 30, 40]

Additionally, is there a specific reason the user needs to specify "Use Values: True" as a setting if "Values" is not an empty string?

@fbergmann
Copy link
Member

I see how this behavior is confusing, but it is the same as in the GUI. I'll discuss it with the team over the next days, whether to fold the output start time into the list of numbers. For now the workaround is to be explicit about it which worked for me with the following snippets:

  • when I was testing it here though, I got a data frame in the first case of [0, 10, 20, 30] for basico.run_time_course(values=[10, 20, 30])
  • to change the start time, I get the expected [10, 20, 30] with basico.run_time_course(values=[10, 20, 30], start_time=10)

The use values parameter is used internally by copasi to decide whether to use the values and persist them, even if briefly changing things. It's the same, that we still have the number of steps / step size values, even if automatic step size is selected. the run_time_course methods should not care about this when you supply values.

@CodeByDrescher
Copy link
Author

I'm using the latest version, but I still get malformed data set when I use the built in-data-handler.

"AutomaticStepSize": False,
"Duration": 30
"Use Values": True,
"Values": [10,20,30]

Without "Output Start Time", I get [0, 20, 30] and with "Output Start Time": 10, I get [20, 30]

Does the output handler use a different system? Or am I still just missing something?

@fbergmann
Copy link
Member

Could you give me a complete example, so i can debug this and ensure i look at the same thing you are looking at. Thank you

@CodeByDrescher
Copy link
Author

Here you go!
demonstration_archive.zip

@fbergmann
Copy link
Member

Thank you so much for this! Just for clarification, is there a reason you prefer to create your own output handler, and calling run_time_course, rather than just calling run_time_course_with_output. No matter what i'll create a new version that solves the issue for you ... might have to wait till monday tho

@CodeByDrescher
Copy link
Author

No worries about the wait, its completely understandable.

We originally used run_time_course_with_output. The only reason we switched is a lack of a corresponding run_steadystate_with_output option, and it was nicer to have a similar initialization pathway rather than two separate paths of logic.

@fbergmann
Copy link
Member

I believe the first issue here is, that in COPASI the "Use Values" field is a str value, and you initialize it with a list of doubles. While I cast the value to a str, that does not improve things. If i explicitly convert your list to str it works for me here

        desired_values = [10.0, 10.5, 11, 11.5, 12, 12.5, 13, 13.5, 14, 14.5, 15, 15.5, 
                        16, 16.5, 17, 17.5, 18, 18.5, 19, 19.5, 20]
        
        # generate string of all the desired values separated by a space
        desired_values = [str(x) for x in desired_values]
        desired_values = " ".join(desired_values)

can you confirm?

@fbergmann
Copy link
Member

We originally used run_time_course_with_output. The only reason we switched is a lack of a corresponding run_steadystate_with_output option, and it was nicer to have a similar initialization pathway rather than two separate paths of logic.

run_time_course_with_output will still be more efficient, but I'll add a method get_values that you can give a set of display names / cns to retrieve their current values. which you could use after a run of steady state.

fbergmann added a commit that referenced this issue Nov 11, 2024
fbergmann added a commit that referenced this issue Nov 11, 2024
fbergmann added a commit that referenced this issue Nov 11, 2024
@fbergmann
Copy link
Member

@CodeByDrescher I've released a new version of basico that should solve all those issues. Now the values specified as desired values will always be taken as the ones to be returned. no need to explicitly set the start value anymore. Also the given values will be converted to a space separated string automatically, so that surprises like the one you encountered wont happen anymore.

I've noticed that i already had a function collect_values that would retrieve values from CN's or display names. So i made this one public now. So you could use that to retrieve results after a run of a steady state computation.

@CodeByDrescher
Copy link
Author

CodeByDrescher commented Nov 18, 2024

Seems like everything works now! I'll be sure to change over to using collect_values when I fully implement steady state down the line. Thank you, Frank!

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

No branches or pull requests

2 participants