Class used in plugin can't be used in a resume run
#23
Comments
|
The |
|
Thanks for the suggestion @pditommaso! I've sadly been unable to fix it with // A class that works like Map, but returns an immutable copy with each method
public class ImmutableMap extends LinkedHashMap implements CacheFunnel {
Map internalMap
ImmutableMap(Map initialMap) {
internalMap = initialMap
}
// Override the methods of the Map interface
@Override
Hasher funnel(Hasher hasher, HashMode mode) {
hasher.putUnencodedChars(internalMap)
return hasher
}
// Rest of the classThis still gives this error in the log: Did I implement it wrong? |
|
Likely the best thing to do is to use https://chat.openai.com/share/b0e9f648-21a6-4069-b474-a4a60e8f334d |
|
We've investigated that and the main problem was that when copying an unmodifiableMap during a |
|
You can see that discussion here: nextflow-io#32 |
|
I think it's not a plugin role to change the semantic of nextflow operator. Therefore you should use usual Map objects instead of |
|
Okay thank you for all the help, too bad there is no way to enforce the immutability of the meta map, but I understand your point on this |
|
@pditommaso is there any other way to achieve what we want here? The immutable maps feature came after @robsyme gave a talk about the dangers of meta map mutability: https://nf-co.re/events/2023/bytesize_workflow_safety We can drop back to a regular map again, but it would be nice to protect users (and devs) from this problem if possible, somehow. |
|
We'll take this into account for DSL3 |
|
Perhaps we can use the @valueobject decoration to automatically implement the required interface. I'll try and test this week. |
|
Hello! |
|
👉 #23 |
|
Paolo - the goal here is not to change in any way how Nextflow's operators work. The only goal here is to construct an object with the following properties:
|
|
I've tried a couple of approaches today and hit some interesting and instructive road blocks 😆 Let's say we want these properties:
Example 1We can put together a very simple example class to illustrate the challenges. We might do something like: import nextflow.util.KryoHelper
class Meta {
@Delegate Map internal = new LinkedHashMap()
static { KryoHelper.register(Meta) }
Object put(Object key, Object value) { internal.put(key, value) }
Meta plus(Map right) { new Meta(internal: internal + right) }
// ... and any other methods (minus, etc)
}
Example 2We can add immutability in a number of different ways, but let's say we go for Nextflow's built-in import nextflow.util.KryoHelper
import nextflow.io.ValueObject
@ValueObject
class Meta {
@Delegate Map internal = new LinkedHashMap()
static { KryoHelper.register(Meta) }
Object put(Object key, Object value) { internal.put(key, value) }
Meta plus(Map right) { new Meta(internal: internal + right) }
}Making the object immutable breaks Kryo serialization:
The Nextflow logs report: Why does is this no longer serializable? Kryo MapSerializerOur This work of incrementally adding new key+value pairs is not supported by our newly immutable class, which is why the Kryo serialization breaks. It's certainly possible to create a custom Serializer class, but I'm starting to think that this immutability feature is starting to feel a little over-engineered. There's a possibility that we're introducing a little too much "magic" - maybe using a vanailla LinkedHashMap class and then providing guidance in documentation is a better approach. |
Why this plugin should use a "magic" object and not just a plain Map? |
|
Because @robsyme gave a nf-core/bytesize talk that put the fear of god into us all about mutable map objects 😆 |
|
Haha. It was certainly not my intention to scare anybody! 😆 Paolo: We had people modifying the map in flight, which can lead to results that depend on task execution timing. For example: workflow {
nums = Channel.of(1..10) | map { [val:it] }
nums
| Foo
| map { meta -> meta.val += 1 }
nums
| VariableProcess
| DoSomethingElse
}In this example, modification of the |
|
I think that implementing the CacheFunnel interface is the easiest path forward, but because the object is also a Map, it's cachefunnel implementation will never be used. I've just opened up a Nextflow PR that would remedy this: nextflow-io/nextflow#4077 |
Bug report
Expected behavior and actual behavior
In the
nf-validationplugin we use an extended class ofLinkedHashMapcalledImmutableMapfor the meta fields. This class works fine when running in a normal run, but gives these warnings during a resumed run and reruns everything:The log shows the following:
Steps to reproduce the problem
You can clone this repository and run it with
nextflow run main.nf. When rerunning the mini pipeline with the-resumeflag, you'll see the errors/warnings.Environment
The text was updated successfully, but these errors were encountered: