Biosimulators Prototype #426
Conversation
Coverage report
Show files with reduced coverage 🔻
Test suite run success125 tests passing in 15 suites. Report generated by 🧪jest coverage report action from 1975799 |
ascibisz
requested review from
interim17,
tyler-foster and
ShrimpCryptid
and removed request for
a team and
tyler-foster
src/simularium/RemoteSimulator.ts
Outdated
| if (smoldynInput !== undefined && smoldynInput !== null) { | ||
| this.webSocketClient.sendWebSocketRequest( | ||
| { | ||
| msgType: NetMessageEnum.ID_START_SMOLDYN, | ||
| fileName: outFileName, | ||
| smoldynInputVal: smoldynInput, | ||
| }, | ||
| "Start smoldyn conversion" | ||
| ); | ||
| } else { | ||
| this.webSocketClient.sendWebSocketRequest( | ||
| { | ||
| msgType: NetMessageEnum.ID_START_SMOLDYN, | ||
| fileName: outFileName, | ||
| }, | ||
| "Start smoldyn conversion" | ||
| ); | ||
| } |
There was a problem hiding this comment.
| if (smoldynInput !== undefined && smoldynInput !== null) { | |
| this.webSocketClient.sendWebSocketRequest( | |
| { | |
| msgType: NetMessageEnum.ID_START_SMOLDYN, | |
| fileName: outFileName, | |
| smoldynInputVal: smoldynInput, | |
| }, | |
| "Start smoldyn conversion" | |
| ); | |
| } else { | |
| this.webSocketClient.sendWebSocketRequest( | |
| { | |
| msgType: NetMessageEnum.ID_START_SMOLDYN, | |
| fileName: outFileName, | |
| }, | |
| "Start smoldyn conversion" | |
| ); | |
| } | |
| this.webSocketClient.sendWebSocketRequest( | |
| { | |
| msgType: NetMessageEnum.ID_START_SMOLDYN, | |
| fileName: outFileName, | |
| smoldynInputVal: smoldynInput ?? undefined, | |
| }, | |
| "Start smoldyn conversion" | |
| ); |
Small nit to maybe make this more DRY.
There was a problem hiding this comment.
Works, looks sound, comments are non-blocking.
I am concerned that the test bed is suffering from a death by a 1000 cuts of small additions to the UI meant to test specific features that either have some small conflict in approach, or otherwise crowd each other.
In this case, it would make more sense to me if we used the select component (where the other trajectories and TEST LIVE MODE API live) to load the smoldyn live mode. Disabling the Run Smoldyn button until that option was selected would make it clear what trajectory or live sim is currently loaded and running. But just an opinion, something to consider going forward. I'd be happy to help with that refactor if you were in agreement it was a good idea?
| <label> | ||
| Initial Rabbit Count: | ||
| <input | ||
| defaultValue="100" | ||
| onChange={(event) => {this.smoldynInput = event.target.value}} | ||
| /> | ||
| </label> | ||
| <button onClick={() => this.loadSmoldynSim()}> | ||
| Run Smoldyn | ||
| </button> | ||
| <br /> |
There was a problem hiding this comment.
Nit: disable/otherwise indicate when a smoldyn-live sim is not loaded in trajectory select field?
src/simularium/RemoteSimulator.ts
Outdated
| public sendSmoldynData(outFileName: string, smoldynInput: string): void { | ||
| this.lastRequestedFile = outFileName; | ||
| if (smoldynInput !== undefined && smoldynInput !== null) { | ||
| this.webSocketClient.sendWebSocketRequest( | ||
| { | ||
| msgType: NetMessageEnum.ID_START_SMOLDYN, | ||
| fileName: outFileName, | ||
| smoldynInputVal: smoldynInput, | ||
| }, | ||
| "Start smoldyn conversion" | ||
| ); |
There was a problem hiding this comment.
Nit: might be worth adding some comments to avoid confusion over loading/running live smoldyn sims and converting smoldyn files.
Edit: after looking at the octopus PR I get the way that converting is kind of baked into the process here, but I still wouldn't mind a comment :)
I agree with this comment, it would have been great to just add the smoldyn test to the selection list of test data if possible. Wonder if/when we would ever remove this test from the test app. |
…pusServicesClient abstraction
ascibisz
force-pushed
the
feature/smoldyn-sim-prototype
branch
from
1975799 to
31cc5dc
Compare
Dependent on this octopus PR
Time Estimate or Size
small
Problem
Initiate a smoldyn simulation via octopus from the example simularium viewer
Link to story or ticket
Solution
Use new message type to initiate a smoldyn simulation via octopus, using an input number (default 100) for the initial rabbit count. After this initial request, proceed like any other simulation.
To trigger this from the example UI, specify desired initial rabbit count in the input field labeled "Initial Rabbit Count:" and click the "Run Smoldyn" button.
At the moment, you need to be running the
feature/prototype-biosimulators-smoldynOctopus branch locally and have the simularium-viewer pointing to it via thenpm run start --localhostnpm run start --localserver. Once this octopus PR is merged, the octopus changes will be in staging, so the run smoldyn prototype will work while running the viewer normally.Type of change
Screenshots (optional):