search

radiasoft / sirepo

Sirepo is a framework for scientific cloud computing. Try it out!
https://sirepo.com
Apache License 2.0
63 stars 31 forks source link

_compute_job_fields expecting model names #1954

Open moellep opened 4 years ago

moellep commented 4 years ago

SRW beamline reports are failing because sim_data/srw.py _compute_job_fields() includes non-model names. To reproduce, visit the SRW beamline page for the CHX example.

robnagler commented 4 years ago

Does this happen in other codes?

moellep commented 4 years ago

Yes, also in warpvnd. I added a work-around with 1a27e6e so it works for now.

robnagler commented 4 years ago

If we ensure that non-model-name values are always a non-string, then this is fine. That's why I put in the assert.

robnagler commented 4 years ago

I think we could use pknested_get in compute_job_hash, and solve all but [beamline[0]['position'], which could be added to pknested_get. For example, propagation.{}.format(str(item['id']))

The values returned by compute_job_fields should be field names.

One case I don't understand is why item.title has to be deleted from an item for it to be included. It is annoying if you recompute based on a title change, but I think it probably often is associated with one of its parameters changing.

Thoughts?

moellep commented 4 years ago

In cases where we really don't want to recompute, we replace the "modelName" with all the compute fields "modelName.f1", "modelName.f2" like in sim_data.warpvnd __non_opt_fields_to_array().

This won't work for non-opt fields which are in a container on data.models - for example, data.models.beamline contains model elements and "title" changes shouldn't require a recompute. That was an explicitly requested feature.

robnagler commented 4 years ago

When you mean "container", do you mean a Python list? We can fix pknested_get to handle that (as it does in BOP).

If title changes shouldn't require a recompute, I think compute_job_hash should exclude all title fields. This would be a complete solution, and could be easily accomplished in one place as opposed to deal with the cases one-by-one. I would think there are other fields (e.g. notes) that shouldn't be included.

moellep commented 4 years ago

There are several standard fields which get ignored when computing the hash, mostly client report fields: aspectRatio, colorMap, notes. Others are app specific like copyCharacteristic and intensityPlotsWidth. "title" could be a common ignore field, although it is only used by SRW.

In several apps, there will be arrays of models. In SRW there is an array of elements stored in data.models.beamline. In elegant/synergia/zgoubi there are arrays named "elements" and "commands".

In SRW, when you are computing a watchpoint in the beamline array, changes of the items in the beamline after the watchpoint should not require a recompute. So you would need to specify the range in the array, something like "beamline[0..5]" rather than just "beamline".