search

OSC / ondemand

Supercomputing. Seamlessly. Open, Interactive HPC Via the Web
https://openondemand.org/
MIT License
263 stars 98 forks source link

Select dropdown not actually selected #3588

Open Xaraxia opened 3 weeks ago

Xaraxia commented 3 weeks ago

Hi team,

I'm having an odd problem with a batch_connect select dropdown. I use this to set some memory and core requirements for desktops. On load, however, it appears to sometimes remember the last value chosen by an end-user. The consequence of this is that it will display one desktop type (for example, my end-user requested the 'xl' desktop) but the values are set to the defaults (which are the values for the 'small' desktop). I have set value to 'small' in the form, but that doesn't seem to be obeyed.

I figured I'd just pop some javascript in that forces the select to be 'small' on load, and this issue would go away. Unfortunately, for some reason, I can't actually set a value to 'selected'. Even if I force it by editing the element in the console, the form doesn't display the selected value. The data-set attributes are working as expected. I just want the value of the form to be correct on load so that the end-users are forced to select the correct value. (Submit is irrelevant, as I'm not actually using the node type - just the generated values.)

For me, it is consistently loading the 'mini' desktop, which is the last one I used. Oddly enough, it is setting the correct values for me when this happens (using Edge for Linux, for Reasons) so I'm suspecting this is a browser-specific problem with the select box. Generated values are below.

<div class="form-group"><label for="batch_connect_session_context_node_type">Select Desktop</label><select class="form-control" name="batch_connect_session_context[node_type]" id="batch_connect_session_context_node_type"><option data-option-for-gpu_type-smallmig="false" data-set-num-cpt="4" data-set-memory="8G" value="mini">Mini: 2 cores (4 cpu threads), 8 GB RAM</option>
<option data-option-for-gpu_type-smallmig="false" data-set-num-cpt="8" data-set-memory="32G" value="small">Standard: 4 cores (8 cpu threads), 32 GB RAM</option>
<option data-option-for-gpu_type-smallmig="false" data-set-num-cpt="16" data-set-memory="64G" value="medium">Medium: 8 cores (16 cpu threads), 64 GB RAM</option>
<option data-option-for-gpu_type-smallmig="false" data-set-num-cpt="16" data-set-memory="128G" value="mediumplus">High-RAM Medium: 8 cores (16 cpu threads), 128 GB RAM</option>
<option data-option-for-gpu_type-smallmig="false" data-set-num-cpt="32" data-set-memory="256G" value="large">Large: 16 cores (32 cpu threads), 256 GB RAM</option>
<option data-option-for-gpu_type-smallmig="false" data-set-num-cpt="48" data-set-memory="512G" value="xl">Extra Large: 24 cores (48 threads), 512 GB RAM</option>
<option data-option-for-gpu_type-smallmig="false" data-set-num-cpt="96" data-set-memory="1000G" value="huge">Huge: 48 cores (96 threads), 1000 GB RAM</option>
<option data-option-for-gpu_type-nogpu="false" data-option-for-gpu_type-l40="false" data-set-num-cpt="12" data-set-memory="90G" value="smallmig">Medium MIG: 6 cores (12 cpu threads), 90 GB RAM</option></select><small class="form-text text-muted"><p>The power (CPU and Memory) available in the desktop. Using more than required may affect your future priority for desktop access.</p>
</small></div>

Another issue is that if we set the smallmig GPU (previous select box) after having chosen 'Huge') above, it changes the select box but not the current value. I'm guessing it's not firing a .change() event?

I'm after something fairly complex here, I realise, and I'm probably going to have to write my own form.js to do what I need, but this didn't behave as I had expected hence the issue. If you can give me some guidance on what might need changing in your javascript to make this work, I could give it a crack.

johrstrom commented 3 weeks ago

Can you share your complete form.yml?

On load, however, it appears to sometimes remember the last value chosen by an end-user

That's odd. It should always display the last chosen value (unless you set cacheable to false)

I suspect there's some data-set directive that's firing to set the node_type but would have to see the full form.yml to be sure.

Xaraxia commented 3 weeks ago

It might actually be selecting always, and I hadn't submitted so didn't trigger that. It's still not doing changes as expected, however. But I do get the right behaviour if after triggering a change in one select box I run $('select').trigger('change') in the console.

I think the fix for this issue is that on load you trigger changes in all select boxes and then when you change one select box you trigger changes in all select boxes. Not ideal from a performance perspective, and you might have a better idea being more familiar with the code, but this should result in all the triggers being fired at any time when the select box might be set.

form.yml below. We currently do not have our own form.js in use here. You shouldn't need the submit script, but let me know if you do.

# Batch Connect app configuration file
#
# @note Used to define the submitted cluster, title, description, and
#   hard-coded/user-defined attributes that make up this Batch Connect app.

---

cluster: "bunya"
title: <%= defined?(@title) ? @title : "Desktop" %>

form:
  - desktop
  - job_name
  - auto_accounts
  - bc_num_hours
  - bc_vnc_idle
  - bc_vnc_resolution
  - working_directory
  - gpu_type
  - node_type
  - queue
  - reservation
  - num_cpt
  - memory

attributes:
  desktop: "mate"
  bc_vnc_idle: 0
  bc_num_slots: 1
  num_cpt:
    widget: hidden_field
    value: 8
  memory:
    widget: hidden_field
    value: "32G"
  bc_vnc_resolution:
    required: true
  queue:
    widget: hidden_field
    value: "cpu_viz"

  auto_accounts:
    help: "Your Bunya account group. The default is usually correct."

  job_name:
    label: "Name of the Job"
    value: "OOD_Desktop"
    help: "Short descriptor that will show in the job queue"

  bc_num_hours:
    label: "Maximum Running Time"
    help: "The maximum number of hours your Desktop session will run for (walltime). This will count towards Fair Share. To release resources, you should shutdown or delete your session when you are done."
    value: 1

  gpu_type:
    label: "Select GPU Type (optional)"
    widget: select
    help: "If you need a GPU, select it here. If in doubt, you don't need one. Note that the desktop will NOT be accelerated."
    options:
      - [ "None", "nogpu", data-set-queue: "cpu_viz" ]
      - [ "L40", "l40", data-set-queue: "gpu_viz" ]
      - [ "GPU:NVIDIA A100 Slice 10GB", "smallmig", data-set-queue: "gpu_cuda", data-set-node-type: "smallmig" ]
    value: "nogpu"

  node_type:
    label: "Select Desktop"
    widget: select
    help: "The power (CPU and Memory) available in the desktop. Using more than required may affect your future priority for desktop access."
    options:
      - [ "Mini: 2 cores (4 cpu threads), 8 GB RAM", "mini", data-option-for-gpu_type-smallmig: false, data-set-num-cpt: 4, data-set-memory: "8G" ]
      - [ "Standard: 4 cores (8 cpu threads), 32 GB RAM", "small", data-option-for-gpu_type-smallmig: false, data-set-num-cpt: 8, data-set-memory: "32G" ]
      - [ "Medium: 8 cores (16 cpu threads), 64 GB RAM", "medium", data-option-for-gpu_type-smallmig: false, data-set-num-cpt: 16, data-set-memory: "64G" ]
      - [ "High-RAM Medium: 8 cores (16 cpu threads), 128 GB RAM", "mediumplus", data-option-for-gpu_type-smallmig: false, data-set-num-cpt: 16, data-set-memory: "128G" ]
      - [ "Large: 16 cores (32 cpu threads), 256 GB RAM", "large", data-option-for-gpu_type-smallmig: false, data-set-num-cpt: 32, data-set-memory: "256G" ]
      - [ "Extra Large: 24 cores (48 threads), 512 GB RAM", "xl", data-option-for-gpu_type-smallmig: false, data-set-num-cpt: 48, data-set-memory: "512G" ]
      - [ "Huge: 48 cores (96 threads), 1000 GB RAM", "huge", data-option-for-gpu_type-smallmig: false, data-set-num-cpt: 96, data-set-memory: "1000G" ]
      - [ "Medium MIG: 6 cores (12 cpu threads), 90 GB RAM", "smallmig", data-option-for-gpu_type-nogpu: false, data-option-for-gpu_type-l40: false, data-set-num-cpt: 12, data-set-memory: "90G" ]
    value: "small"

  working_directory:
    label: "Working Directory"
    value: null
    help: "Directory that the job will start in. Defaults to home directory if left blank."

  reservation:
    label: "Reservation"
    value: null
    help: "Reservation name. Do not use unless instructed."
johrstrom commented 3 weeks ago

Thanks! I'll take a look at this when I get a chance.

I think the fix for this issue is that on load you trigger changes in all select boxes and then when you change one select box you trigger changes in all select boxes.

This should be happening. I'm not ruling out a bug, but I'm quite sure data-set handlers run when the page loads so that the initial page gets set to the appropriate state. But again, there could always be a bug!

johrstrom commented 3 weeks ago

I'm having trouble replicating on OnDemand 3.1.4/Windows/Firefox.

On load, however, it appears to sometimes remember the last value chosen by an end-user.

It should always populate the previous values, even for hidden_fields. You can set cacheable: false in the attributes section to turn this off. This means that form field specifically will never be cached and displayed on first page load.

The consequence of this is that it will display one desktop type (for example, my end-user requested the 'xl' desktop) but the values are set to the defaults (which are the values for the 'small' desktop)

I cannot replicate this. From reading this sentence, it seems like this is what you're saying what I've given below. Please confirm this is the steps to reproduce, the expected and actual behavior. Again, I can't reproduce, so the actual behavior for me at least, is the expected behavior.

Steps to reproduce:

Expected behavior:

Actual behavior (for me it is the expected behavior, but I assume this is the actual behavior for you):

For me, it is consistently loading the 'mini' desktop, which is the last one I used.

This is expected, again because of the caching.

Oddly enough, it is setting the correct values for me when this happens (using Edge for Linux, for Reasons) so I'm suspecting this is a browser-specific problem with the select box.

Might could be, especially if your end user is on Mac and using Safari. I can run Windows and Linux, but will have to reach out to a colleague who runs Mac.

johrstrom commented 3 weeks ago

Hi I just want to be sure I don't lose the thread on this. I can't replicate, but that doesn't mean there's nothing here - I could be missing something for sure.

Let me know if the steps to reproduce/expected and actual behavior I've given above is correct. And if it's a Mac/Safari user I can try to pull a colleague in to help.

Xaraxia commented 2 weeks ago

Hi,

Sorry I vanished, my darling son brought COVID home and I'm still a bit smashed.

End-user is Windows 11 and Firefox 126.0.1

I was having an issue where the values weren't being set on first load of the form, so people were accepting the standard desktop without the values being set. This is why I set "value". I assumed that the automatic save would override that but given what you have said I will move the default to the submit.yml instead so that I don't hard-set the value in the form.

However, I'm also having an odd issue in another desktop where it is ignoring the entered value and doing my default instead. I'm wondering whether the actual problem isn't the form at all, but rather the submit script. Is re-assigning memory like this a problem?

Ergo, below is my submit script:

<%-
  cpt = num_cpt.blank? ? 8 : num_cpt.to_i
  qos = qos_name.blank? ? "normal" : qos_name
  memory = memory.blank? ? "32G" : memory
  gtype = gpu_type == "nogpu" ? "" : gpu_type.downcase
  gres = gtype == "smallmig" ? "nvidia_a100_80gb_pcie_1g.10gb" : gtype
%>
---
batch_connect:
  template: vnc
  min_port: 5920
  password_size: 8
script:
  accounting_id: "<%= auto_accounts %>"
  queue_name: "<%= queue %>"
  native:
    - "--cpus-per-task=<%= cpt %>"
    - "--tasks-per-node=1"
    - "--nodes=1"
    - "--qos=<%= qos %>"
    - "--mem=<%= memory %>"
    - "--job-name=<%= job_name %>"
  <%- unless gres.blank? %>
    - "--gres=gpu:<%= gres %>:1"
  <%- end -%>
  <%- unless reservation.blank? %>
    - "--reservation=<%= reservation %>"
  <%- end -%>
Xaraxia commented 2 weeks ago

On the assumption that one of the things in my prior comment was the issue, I have made modifications and tested:

On load, am seeing the following: image

Note from above that when 'smallmig' is selected, the others shouldn't show in the dropdown. So, it loaded the correct value, but didn't trigger the hide of the other values.

However, I can confirm that having the re-assigning of values in the ERB plus the preset values was causing the other problems. So, this specific issue is mostly not an issue. Just need to trigger the other selects on load to do what is expected and I think we're golden.

Thanks for your patience, that was a silly error on my part.

Xaraxia commented 2 weeks ago

By the way, is there a way to always choose the 'Standard' desktop as a default? I'd prefer it didn't save the previous one. Convenient for the users, yes, but we're trying to encourage them to use as little resources as possible and a lot of our users just click through.