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

add timeunits to simList show method #260

Closed
eliotmcintire opened this Issue Feb 12, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@eliotmcintire
Copy link
Contributor

eliotmcintire commented Feb 12, 2016

There is currently not a STRONG enough message showing the time units of the modules. Because of modularity, the user doesn't need to know that different modules have different time units. SpaDES takes care of that. BUT, if time units are not set properly in the meta data, then there is currently no error/warning that would make it obvious that this is the problem.

It seems like it would be too slow to add a column to the events list, and it is not entirely appropriate, since they are all being shown in a single unit anyway ( timeunit(sim) units).

Some possibilities

  • show the timeunits(sim) just above the events list in the show method for simList, so that if debug is on, then it will be generally visible to a user in their console
  • provide a message, say at the simInit call that says that SpaDES will be converting timeunits if the modules are in different units.

I don't like messages because they will show up all the over and over again; and I don't like the showing of the timeunits(sim) because the user might not see it.

We need a balance between too much information and not enough information.

Thoughts?

@achubaty

This comment has been minimized.

Copy link
Contributor

achubaty commented Feb 12, 2016

My plan was to add it to the show method of the simlist, and also create a
new function that pulls that info for each module from the metadata if a
character vector of package names is passed or from the simlist object if
one is passed. This way the user doesn't need to create a simlist object in
order to easily get at that info.

On Friday, 12 February 2016, Eliot McIntire notifications@github.com
wrote:

There is currently not a STRONG enough message showing the time units of
the modules. Because of modularity, the user doesn't need to know that
different modules have different time units. SpaDES takes care of that.
BUT, if time units are not set properly in the meta data, then there is
currently no error/warning that would make it obvious that this is the
problem.

It seems like it would be too slow to add a column to the events list, and
it is not entirely appropriate, since they are all being shown in a single
unit anyway ( timeunit(sim) units).

Some possibilities

  • show the timeunits(sim) just above the events list in the show
    method for simList, so that if debug is on, then it will be generally
    visible to a user in their console
  • provide a message, say at the simInit call that says that SpaDES
    will be converting timeunits if the modules are in different units.

I don't like messages because they will show up all the over and over
again; and I don't like the showing of the timeunits(sim) because the user
might not see it.

We need a balance between too much information and not enough information.

Thoughts?


Reply to this email directly or view it on GitHub
#260.

Alex Chubaty

@achubaty

This comment has been minimized.

Copy link
Contributor

achubaty commented Feb 15, 2016

currently, one can use:

# for a module, read the file directly
moduleMetadata('module', 'path/to/modules/dir')$timeunit

# for a simulation, read the simList object
timeunits(mySim)

this means we don't need to create new functionality, just display the info more prominently.

the show method for a simList currently displays the modules, so I will add the timeunits to this display.

achubaty added a commit that referenced this issue Feb 15, 2016

@achubaty achubaty closed this Feb 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.