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

Implement an autostop mode for gruffi #7

Merged
merged 17 commits into from
Feb 13, 2024
Merged

Implement an autostop mode for gruffi #7

merged 17 commits into from
Feb 13, 2024

Conversation

heylf
Copy link
Contributor

@heylf heylf commented May 4, 2023

Dear @vertesy,
I took the liberty from my previous question and implemented an automatic mode for gruffi. Please check if that is to your liking. I took a route which does not require too much reformatting and simply implemented an argument (atuostop) to trigger a button click after the app is initialized.

I also updated the manual for the Shiny.GO.thresh function to include the autostop option.

The code was tested it on my machine (Ubunut 22.04) and it works with R 4.3.0.

It requires the package shinyjs !! First I wanted to let you check the code before I also include the package in the installation.

Cheers,
Florian

@vertesy
Copy link
Collaborator

vertesy commented May 12, 2023

Thank you & sorry, been very busy, but I try to get back on this soon!

@heylf
Copy link
Contributor Author

heylf commented May 15, 2023

The autostop is not completly automatic yet because you still have to run a browser in order to stop the application. Thus, if you execute the program on a cluster (e.g., IBM) the script freezes. Consequently, I tried a few things, but an implementation without the execution of a browser is not trivial.

@heylf
Copy link
Contributor Author

heylf commented May 15, 2023

Do not merge the PR I will work on this!

@heylf
Copy link
Contributor Author

heylf commented May 15, 2023

So. I have added a total new function for an automatic setting. Took code parts from the server.R and gruffi.R. This will not run the Shiny.app anymore, but has the same functionality. Yet, @vertesy please check if I have not made any mistakes.

@liezeltamon
Copy link

Hi!

The added function Auto.GO.thresh() is super useful and just wondering when you are planning to merge this? Thank you!

@vertesy vertesy merged commit a7fb02f into jn-goe:main Feb 13, 2024
@vertesy
Copy link
Collaborator

vertesy commented Feb 13, 2024

Hey,

I am very sorry, for the late follow up. I now checked the code and will pull. Will test and probably further modify.

Of note, I did a major update of the package (bf, cleanup).

@vertesy
Copy link
Collaborator

vertesy commented Feb 14, 2024

I went over the new function from this PR, and will commit the changes soon.

  • I massively cleaned up the Auto.GO.thresh() the code. The code was very verbose (largely inherited from Shiny.GO.thresh(), that is cleaned up as well.)
  • The original PR was missing the threshold assignments to @misc
  • Variable names were often misleading (again, inherited and fixed in both places).
  • Removed parts that were not used
  • Code restructuring / organization
  • Some code annotation

I will now add a new feature for visualization of intermediate results.
One reason for not implementing originally an Auto.GO.thresh() is to force people to look at the results and understand what happens, instead of blindly taking a T/F column.
I understand that for automated workflows this is a problem. Therefore with some more feedback Auto.GO.thresh() is now part of the package.

Thank you for the contribution, much appreciated!

Remaining issues

Shiny.GO.thresh() seems to round up threshold values, wheread Auto.GO.thresh() does not. I added a signif() but it's still not the same.
Note that rounding may lead, in very rare cases, suboptimal results. Will remove from both.

Auto.GO.thresh()

$thresh.stress.ident1
[1] 0.690027

$thresh.stress.ident2
[1] 0.71495

$thresh.notstress.ident3
[1] 0.672001

Shiny.GO.thresh()

$thresh.stress.ident1
[1] 0.69

$thresh.stress.ident2
[1] 0.715

$thresh.notstress.ident3
[1] 0.672

@liezeltamon
Copy link

thank you both! hope to try this soon

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.

3 participants