[ENH] Add res and den keywords to indicate resolution of resampled data

Covers the use case when one would like to have several resamplings of a derivative (i.e., keep high-res and low-res versions of data).
cc/ @bids-standard/derivatives

Would this also apply to temporal resampling? That can be done both for 4D nifty BOLD, and electrophys?

@robertoostenveld
Would this also apply to temporal resampling? That can be done both for 4D nifty BOLD, and electrophys?

The intent here is to describe only spatial samplings. Feel free to suggest modifications to represent temporal samplings too or a new keyword (time-?).
Another question is whether there is a use case where you have more than one time-samplings. I can see pipelines having both (spatial) high- and low-resolution pathways, but I haven't seen such a thing with the time dimension of fMRI.

Why no "space" keyword in the CIFTI example?

Following the spec, that would mean data remain in the original BOLD "space". And that is completely correct regardless of from where and how you brought in the prior knowledge (i.e., the location of volumetric and surface sampling positions). For simplicity, I preferred to keep the concept of space away.

isn't space-fsaverage still required in the filename or the .json?

Following the previous point, space-fsaverage here would be a misrepresentation of the data

Also - the CIFTI example should also remove the reference template (space) from the densitiy description?

I can add the language Matched with as I did in previous examples, if that feels clearer. The intent of this description is to quickly tell how densely you sample the manifold ("what we are looking at? - a surface of 41k points per hemisphere with one corresponding fMRI time series per vertex"), it is not to keep track of the provenance ("okay, this is 41k vertices per hemisphere because it comes from fsaverage6").

The intent here is to describe only spatial samplings. Feel free to suggest modifications to represent temporal samplings too or a new keyword (time-?).
Another question is whether there is a use case where you have more than one time-samplings. I can see pipelines having both (spatial) high- and low-resolution pathways, but I haven't seen such a thing with the time dimension of fMRI.

It is the principle rather than the specific topic of resampling along a specific dimension: I believe that keywords limited to one modality are better not be part of common derivatives, but be under the modality specific derivatives. Otherwise all keywords of all modalities (including future BEPs for other data types) should be documented in common derivatives, which would become a mess.
Common derivatives should describe metadata that is common and that - in principle - could apply to all modalities. E.g. the operating system on which the pipeline ram, the software details, component versions, the type of CPU or GPU,  etc. Hence I would expect derivatives/01-introduction.md to be relatively small and derivatives/02-mri.md more elaborate.

This addition applies to PET/SPECT, CT, derived contrast maps, or any 3D
object with samples in a regular grid or a surface..
On Fri, Aug 16, 2019, 01:35 Robert Oostenveld<EMAIL_ADDRESS>wrote:

The intent here is to describe only spatial samplings. Feel free to
suggest modifications to represent temporal samplings too or a new keyword (
time-?).
Another question is whether there is a use case where you have more than
one time-samplings. I can see pipelines having both (spatial) high- and
low-resolution pathways, but I haven't seen such a thing with the time
dimension of fMRI.
It is the principle rather than the specific topic of resampling along a
specific dimension: I believe that keywords limited to one modality are
better not be part of common derivatives, but be under the modality
specific derivatives. Otherwise all keywords of all modalities (including
future BEPs for other data types) should be documented in common
derivatives, which would become a mess.
Common derivatives should describe metadata that is common and that - in
principle - could apply to all modalities. E.g. the operating system on
which the pipeline ram, the software details, component versions, the type
of CPU or GPU
https://journals.plos.org/plosone/article?id=10.1371/journal.pone.0038234,
etc. Hence I would expect derivatives/01-introduction.md to be relatively
small and derivatives/02-mri.md more elaborate.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/bids-standard/bids-specification/pull/301?email_source=notifications&email_token=AAESDRTZJHAUJKKXKFG4M4DQEZRGLA5CNFSM4ILNEIIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4OBDWI#issuecomment-521933273,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAESDRX7XFMFNAMRBHTCXATQEZRGLANCNFSM4ILNEIIA
.

This addition applies to PET/SPECT, CT, derived contrast maps, or any 3D object with samples in a regular grid or a surface..
…

yes, which means that right now it only covers 1 of the 5 modalities in the specification  (since CT and PET are not yet part of the specification).
I can see the point that some derivative extensions apply to more than one (i.e. MRI, PET and CT; we would also have that with MEG, EEG and iEEG, and I would also expect BEH and EYE to have that overlap. So common-imaging, and common-electrophys and common-behavioral make sense. But common-to-all should be distinguished from imaging.

This addition applies to PET/SPECT, CT, derived contrast maps, or any 3D object with samples in a regular grid or a surface..
…

Yes, which means that it only applies to 1 out of the 5 modalities that is presently included in the specification (PET and CT are not yet).
It makes sense to have  common-imaging-derivatives (for MRI, PET and CT), common-electrophys-derivatives (for MEG, EEG and iEEG) and common-behavioral-derivatives (for BEH and eyetracking), but common-for-all-derivatives should not prescribe details for either one, unless it really applies to that derivative.

Okay, so it sounds like @robertoostenveld's primary concern here is with the res keyword being specifically spatial, as temporal resolution is the resolution of interest in electrophysiological analysis. Would it be reasonable to recast this res as sres for spatial resolution (or vres for volumetric resolution) and save tres for temporal resolution?
They're not quite as readable (especially tres, which is an actual word...), so I'm happy to hear alternative suggestions.
Another option is for res- to have different meanings in different modalities, but this would require it to be the case that you won't generally want to combine data across modalities and indicate that you have, for example, estimated a 50Hz time series in a 1mm isotropic voxel grid. I don't know how likely that would be, so I can't really issue a preference.

Common derivatives should describe metadata that is common and that - in principle - could apply to all modalities. E.g. the operating system on which the pipeline ram, the software details, component versions, the type of CPU or GPU, etc. Hence I would expect derivatives/01-introduction.md to be relatively small and derivatives/02-mri.md more elaborate.

@robertoostenveld I'm going to be reworking the MR-specific sections of common derivatives to make just this sort of distinction. I'll ping all derivatives folks to have a look when I have done.

I don't have a preference for a specific name or so, and don't object to res itself. It is just that it should be clear that the keyword applies only to imaging data, not to behavioral or electrophys. I have had many electrophys people expressing confusion about BIDS documentation that is MRI biased, asking whether fields like acq, rec, dir, mod etcetera also apply for their data.
Oh, now that I think of it, we put quite some effort into making this table, not only to document their possible presence and order, but also to ensure consistency between similar types (like EEG and iEEG). The table is relevant to keep in mind for mapping keys (as in <key>-<value) to datatypes/formats.
Regarding the res equivalent for electrophys: that is something to discuss with the electrophys folks, but I expect them to phrase this as "resampling" rather than "changing the resolution", and  that the value would rather be expressed in Hz (i.e. rate) rather than seconds (i.e. duration between samples). I.e., I suspect fsample to be preferred by the electrophys field.
If temporal resampling were an issue for processing 4D NIFTI (which it is not at the moment), then it would not surprise me if fMRI folks would prefer that to be expressed in seconds (duration between samples, i.e. TR) rather than Hz (rate). In that case I would also be fine with another keyword than fsample for derived 4D bold data.
This example also makes me realize that the proposed res could not apply to the time axis any way, since the <value> must be specified in mm. ... or would the value be a string including the units? How about the value for den? If the units are not specified in the value, where would they be specified? Here I am thinking e.g. about high-res MRI at 100um or CT at 30um. Units, see also the appendix are tricky to get documented properly.

The res and den keywords apply to any 2D and 3D data, regardless of the origin. den could be used, for example, in EEG data if having two processing paths where one uses the full array of receptors and the other some subsample. And res could have a use in non-parametric solutions to the inverse problem in EEG. Resolution/surface density alludes to signal processing concepts rather than modalities. We are talking of the 80 part of the 80/20. No way this is a 5% use case.

This example also makes me realize that the proposed res could not apply to the time axis any way, since the  must be specified in mm. ... or would the value be a string including the units? How about the value for den? If the units are not specified in the value, where would they be specified? Here I am thinking e.g. about high-res MRI at 100um or CT at 30um. Units, see also the appendix are tricky to get documented properly.

This is not what has been proposed here. The proposed spec is res-<index> and index maps to a dictionary where index are the keys and the values are just descriptions. Similarly for den-<label>. The units must be encoded in the file format metadata.
Regarding tres/fsample: this is why I asked for examples at first. Although I don't see the use (this key is for disentangling resolutions when derivatives with several of them must be generated). If only one resolution is present, then that was previously well captured by the spec.
My fear here is that dividing the common derivatives again by broader data types will really extend the already long time it is taking for us to get this finished.

I believe that HCP publish source reconstructed MEG derivatives as CIFTI files. For these images the res and den  would definitely apply..
I second the idea that the temporal resolution is also something that can be modified in many workflows (esp EEG) - but also physio confounds from MR and stimulus presentation.. So it does make sense to add a general keyword for it... CIFTI file format uses "timestep" to refer to this in their metadata..

@edickie
I second the idea that the temporal resolution is also something that can be modified in many workflows (esp EEG) - but also physio confounds from MR and stimulus presentation.. So it does make sense to add a general keyword for it... CIFTI file format uses "timestep" to refer to this in their metadata

I'm also totally in favor of adding a keyword for temporal resolution. The broader question is whether that keyword is going to be ever used. If EEG (or whatever modality) do not play with several resolutions at a time, then they are not necessary.
My thinking behind res (and probably with even more reasons den) is to allow fMRIPrep users to generate preprocessed data:

in MNI space; and
with two versions: one keeping the original resolution, one with the resolution matching MNI space.
In such a situation, the only option is to use desc to differentiate those two (and drop the desc-preproc we currently use). Although using desc for this seems pretty inappropriate.

The discussion is already too involved. But I have a couple of broad questions:

what is the criteria for something to be added as a keyword rather than a key in the sidecar with a new desc label?

And res could have a use in non-parametric solutions to the inverse problem in EEG. Resolution/surface density alludes to signal processing concepts rather than modalities. We are talking of the 80 part of the 80/20. No way this is a 5% use case.

what is the criteria for deciding if something is 80 or 20? :-) In MEG source modeling, the canonical way we talk about resolution is in source spaces and how many times it has been subdivided rather than in terms of mm. See here for instance: https://martinos.org/mne/stable/manual/cookbook.html?highlight=ico5#anatomical-information. But this is surface source spaces and probably doesn't apply to CIFTI/NIFTI but I can't say from my superficial read of the PR.

what is the criteria for something to be added as a keyword rather than a key in the sidecar with a new desc label?

Simplest answer I can think of is: By default, a metadata element should go to the side car; only becomes a filename key if (1) the element is needed to avoid two distinct files having the same name, or (2) the element is simply so fundamental that use of the file would be impaired if omitted.

Right, however you could use desc to distinguish between files having the same name. From the specification:

The desc keyword is a general purpose field with freeform values. To distinguish between multiple different versions of processing for the same input data the desc keyword should be used.

@jasmainak In MEG source modeling, the canonical way we talk about resolution is in source spaces and how many times it has been subdivided rather than in terms of mm.

Then the proposed den (density) keyword applies to MEG. As I mentioned above, the concept of units is irrelevant as these keywords can only be used when there are two or more resolutions/densities as a result of your pipeline. E.g., your pipeline generates a derivative with two densities: 'oct6' and 'ico5'. This would allow these two files representing two "twin" objects of the same data type, and same all other properties except for the sampling, to be called den-4k and den-10k. Please look at the examples for surfaces in this PR.

@jasmainak what is the criteria for deciding if something is 80 or 20? :-)

I don't think there is. I just meant that, if I were asked what the coverage (or application) of this PR is then I'd say it falls on the 80 side because I believe this is necessary for imaging data and potentially for many other modalities. I'd see this a 20-side if it applied only to MRI, for instance.
This PR applies to any nonparametric 2D and 3D sampled data regardless of the modality. Happy to extend it to cover 1D or ND+t if we can foresee the utility, and to find alternative names to the keywords if they misrepresent some modality.

@jasmainak  Right, however you could use desc to distinguish between files having the same name.

Can you give an example of how would you use desc here? - We discussed this at large in the BIDS meeting last year. The impression I got is that desc should be avoided if a keyword in a controlled vocabulary can be defined to cover some specific and relevant concept (e.g., space). Otherwise, we should drop space and, since several desc are not allowed (I believe), then end up with monstruosities like desc-MNI152NLin6Asym2mmBrainSmoothed.
I propose this after a large discussion in the area of imaging templates, where several versions of the template (which differ in resolution) can be used. If space is an entity (physical world), I thought resolution/density should have their entity (digital world).

BTW, with the changes of the latest commit I think res could be used for timeseries too.

the changes + examples seem fine to me.
regarding the desc label, it probably ties into the discussion here: https://github.com/bids-standard/bids-specification/issues/309 where @satra quotes discussion from the derivative meeting:

filename globbing is not a recommended pattern outside of the base filename

I don't quite agree with the above statement. But if this were true, then desc-MNI152NLin6Asym2mmBrainSmoothed shouldn't matter since you use it only to disambiguate and the real information is in the sidecar.
That said, I don't want to block this PR any further if @robertoostenveld and others are okay with the changes.

In MEG source modeling, the canonical way we talk about resolution is in source spaces and how many times it has been subdivided rather than in terms of mm. See here for instance: https://martinos.org/mne/stable/manual/cookbook.html?highlight=ico5#anatomical-information.  But this is surface source spaces and probably doesn't apply to CIFTI/NIFTI but I can't say from my superficial read of the PR.

Yes, this does also apply to MEG source results in CIFTI (which I happen to have co-authored) and to HCP. In HCP we share processed results both as "dense" (i.e. regular volumetric for beamforming, surface-based for minimum norm estimates) and as "parcellated" (esp. for the connectivity matrices). The res and den apply to the spatial aspect of the dense representation in CIFTI.
In HCP we don't consider source reconstructions as (merely) pre-processed, but as (fully) processed, since a lot decisions have to be made in the source-level processing that fundamentally affect the output. See this figure for HCP-MEG: the functional preprocessing is considered to be the box at the top (shared between task and rest) and the anatomical preprocessing the box at the left. Res and den play a role in the box at the left, and thereby also affect the functional outcomes.
Similar as other key-value pairs that we tried to make consistent between modalities, we should do the same for 'res' and 'den'. They would apply to MEG/EEG source level results- the output of a multi-modal analysis with both MRI and electrophys  - but that does not mean we have to stretch res and den to EEG-only datasets like this one from @sappelhoff.

Should the common derivatives description explain that derived data can be of the same type, but also might be of an other type than the 8 types listed for BIDS-raw in the common principles?
5. Data type - a functional group of different types of data. In BIDS we define eight data types: func (task based and resting state functional MRI), dwi (diffusion weighted imaging), fmap (field inhomogeneity mapping data such as field maps), anat (structural imaging such as T1, T2, etc.), meg (magnetoencephalography), eeg (electroencephalography), ieeg (intracranial electroencephalography), beh (behavioral).

I have merged common-derivatives into this branch. @oesteban please verify that this still reflects your intended change.
Could all participants in this discussion re-review this? It was refactored significantly due to the separation of imaging derivatives.

@oesteban @edickie What's the status of this conversation?

Why no "space" keyword in the CIFTI example?

Following the spec, that would mean data remain in the original BOLD "space". And that is completely correct regardless of from where and how you brought in the prior knowledge (i.e., the location of volumetric and surface sampling positions). For simplicity, I preferred to keep the concept of space away.

isn't space-fsaverage still required in the filename or the .json?

Following the previous point, space-fsaverage here would be a misrepresentation of the data

Also - the CIFTI example should also remove the reference template (space) from the densitiy description?

I can add the language Matched with as I did in previous examples, if that feels clearer. The intent of this description is to quickly tell how densely you sample the manifold ("what we are looking at? - a surface of 41k points per hemisphere with one corresponding fMRI time series per vertex"), it is not to keep track of the provenance ("okay, this is 41k vertices per hemisphere because it comes from fsaverage6").

Now that fsLR is in common derivatives, should we add a space to the CIFTI files?

@robertoostenveld @satra @emdupre Any remaining issues on your end before merging to common-derivatives?

@effigies - without a reference to  space in the cifti json file, how would i know the underlying geometry of the cifti file, both the surface part and the volume part?

@satra If I understand Common file level metadata fields correctly, if space- is a standard template, then that defines it. If it is not, then it requires a SpatialReference metadata field.
@oesteban?

@robertoostenveld @satra @emdupre Any remaining issues on your ends before merging to common-derivatives?

I had a look at https://bids-specification.readthedocs.io/en/common-derivatives/05-derivatives/01-introduction.html and the other two pages.
In the "Introduction" I am not so happy with the phrasing SpatialReference = REQUIRED when a custom template or file is used.
I would prefer REQUIRED in case a custom reference image was used.
Other than that I am fine with it. From the @bids-standard/derivatives-electrophys  perspective I believe that the common derivatives and the anatomical MRI will allow a further/future specification for derivatives of that data as well.

I'm fine with @robertoostenveld suggestion. Will update including that, when https://github.com/bids-standard/bids-specification/pull/301#discussion_r356308259 is more clear - if everyone is :+1: with the suggestion.

Unfortunately, my computer crashed with my notes from our meeting last week and I did not use a program with auto-save, so if anybody has those, feel free to share.
The upshot of the meeting, however, was that I would be working to put together a concrete use case and a tool that would allow us to go from a simple JSON representation of the relevant spaces to a sparse, but usable, Connectome-Workbench-style .wb.spec file.
To this end, I've processed some data with fMRIPrep 20.0.0 with --cifti-output enabled, and will be manually augmenting with a JSON sidecar.
fmriprep/sub-01
├── anat
...
├── func
...
│   ├── sub-01_task-repeatchange_run-1_space-fsLR_den-91k_bold.dtseries.json
│   ├── sub-01_task-repeatchange_run-1_space-fsLR_den-91k_bold.dtseries.nii
...

Currently the JSON file contains:
{"space": "HCP grayordinates", "surface": "fsLR", "volume": "MNI152NLin6Asym", "surface_density": "32k", "grayordinates": "91k"}

As this is a case where the image is not part of the outputs, it makes sense to use URLs to indicate where a file can be retrieved. I would suggest making fmriprep/space-fsLR_den-91k_bold.dtseries.json:
{
    "SpatialReference": {
        "VolumeAll": "https://github.com/templateflow/tpl-MNI152NLin6Asym/blob/796ab78/tpl-MNI152NLin6Asym_res-02_T1w.nii.gz",
        "CortexLeft": "https://github.com/templateflow/tpl-fsLR/blob/f82f453/tpl-fsLR_hemi-L_den-32k_midthickness.surf.gii",
        "CortexRight": "https://github.com/templateflow/tpl-fsLR/blob/f82f453/tpl-fsLR_hemi-R_den-32k_midthickness.surf.gii"
    }
}

For now I'll assume that URLs are acceptable, and say this should translate into:
<?xml version="1.0" encoding="UTF-8"?>
<CaretSpecFile Version="1.0">
   <MetaData>
   </MetaData>
   <DataFile Structure="CortexLeft"
             DataFileType="SURFACE"
             Selected="true">
      https://github.com/templateflow/tpl-fsLR/blob/f82f453/tpl-fsLR_hemi-L_den-32k_midthickness.surf.gii
   </DataFile>
   <DataFile Structure="CortexRight"
             DataFileType="SURFACE"
             Selected="true">
      https://github.com/templateflow/tpl-fsLR/blob/f82f453/tpl-fsLR_hemi-R_den-32k_midthickness.surf.gii
   </DataFile>
   <DataFile Structure="Invalid"
             DataFileType="VOLUME"
             Selected="true">
      https://github.com/templateflow/tpl-MNI152NLin6Asym/blob/796ab78/tpl-MNI152NLin6Asym_res-02_T1w.nii.gz
   </DataFile>
</CaretSpecFile>

A second, per-subject use case might look like fmriprep/sub-01/func/sub-01_bold.dtseries.json:
{
    "SpatialReference": {
        "VolumeAll": "../anat/sub-01_desc-preproc_T1w.nii.gz",
        "CortexLeft": "../anat/sub-01_space-fsnative_hemi-L_den-32k_midthickness.surf.gii",
        "CortexRight": "../anat/sub-01_space-fsnative_hemi-R_den-32k_midthickness.surf.gii"
        }
}

and
<?xml version="1.0" encoding="UTF-8"?>
<CaretSpecFile Version="1.0">
   <MetaData>
   </MetaData>
   <DataFile Structure="CortexLeft"
             DataFileType="SURFACE"
             Selected="true">
      ../anat/sub-01_space-fsnative_hemi-L_den-32k_midthickness.surf.gii
   </DataFile>
   <DataFile Structure="CortexRight"
             DataFileType="SURFACE"
             Selected="true">
      ../anat/sub-01_space-fsnative_hemi-R_den-32k_midthickness.surf.gii
   </DataFile>
   <DataFile Structure="Invalid"
             DataFileType="VOLUME"
             Selected="true">
      ../anat/sub-01_desc-preproc_T1w.nii.gz
   </DataFile>
</CaretSpecFile>

Before I get working on a tool, does this seem reasonable to everybody? I'm using the T1w above, but possibly dsegs make more sense?

Following a meeting with @satra, @oesteban and @mgxd, we established the following:

wb_view will take URLs, as long as those URLs resolve to valid GIFTI/NIfTI files. I have separately confirmed that they will take relative paths, as well.
Any conventions promoted here should be made with reference to the CIFTI-2 specification, not conventions followed by ConnectomeWB. It is the responsibility of tools to convert between these, when necessary.
Additional measures should be taken on the TemplateFlow fMRIPrep side as reference implementations to ensure that good URLs that point directly to the necessary GIFTI (Conte69) and NIfTI (MNI152NLin6Asym) can be retrieved programmatically. This is not necessary for the spec work, but valid and persistent URLs will make examples more valuable.

I will try to polish up this pull request ASAP, which might need to wait until late next week based on my schedule.
Please add any notes you think are important.
cc @sappelhoff @franklin-feingold Status update.

@satra Reading the CIFTI-2 spec, it looks like we have neither CortexLeft or CORTEX_LEFT, but rather CIFTI_STRUCTURE_CORTEX_LEFT. Do you think we could say "BrainStructure values with the prefix CIFTI_STRUCTURE_ omitted` or simply that we suggest using "BrainStructure values from page 10 of the CIFTI-2 specification"?
I plan to finish drafting this in the next 1-2 days.

@effigies - don't know a good answer. if every future structure starts with that prefix, then sure. but in the absence of any guarantees seems like aligning to the CIFTI -2 spec in this case makes sense.

@edickie @satra @oesteban @robertoostenveld Can you please review this PR when you get a few minutes? If there are any conceptual issues rather than just drafting issues, can we schedule a call ASAP?

Love it so far! I have one more clarification question re: inheritance principle. For the case of spaces - we could have a top level json called:
space-fsLR_den-91k_bold.json
space-fsLR_den-91k_thickness.json
sub-01/
    anat/
        sub-01/anat/sub-01_hemi-L_space-fsLR_den-91k_thickness.shape.gii
    func/
         sub-01_task-rest_space-fsLR_den-91k_bold.dtseries.nii

or
space-fsLR_den-91k.json
sub-01/
    anat/
        sub-01/anat/sub-01_hemi-L_space-fsLR_den-91k_thickness.shape.gii
    func/
         sub-01_task-rest_space-fsLR_den-91k_bold.dtseries.nii

Do we need to clarify this in an extra example?

@edickie...

space-fsLR_den-91k.json
sub-01/
    anat/
        sub-01/anat/sub-01_hemi-L_space-fsLR_den-91k_thickness.shape.gii
    func/
         sub-01_task-rest_space-fsLR_den-91k_bold.dtseries.nii

Do we need to clarify this in an extra example?

Are sidecars without suffixes permitted? I can't think of an example within the rest of the spec or derivatives off the top of my head. My inclination is that we don't need this, but happy to be wrong, or to include something you suggest.

I think the final two comments can be summarized:

We may want to add discussion of top-level sidecars for SpatialReference.

My overall feeling is that this is a general principle, and unless we're introducing something new WRT inheritance, we don't have a particular need to duplicate this in the example. I don't think we do that here.

We could use VOLUME instead of VolumeReference.

Agreed, but for reasons outlined in https://github.com/bids-standard/bids-specification/pull/301#discussion_r428960886, I don't see a strong argument for it.

I think that making these changes, if the community deems them necessary, would be sufficiently small as to be doable on the main PR (#265). To get things moving on the final review, I'm going to merge shortly.


Add Django 2.0 to TravisCI test matrix

Add Django 2.0 with Python 3.5 and 3.6 to the test matrix. Currently in alpha, scheduled for December 2017 release.

Coverage remained the same at 68.326% when pulling 365271f38d232144a93768b0a400bab31ab5cc3d on jwhitlock:django-2.0 into 1873ab778d18e89fbffbdb73019f5c56bd012088 on django-nose:master.


Reopen standard handles when they are a TTY

Reopening the TTY prevents crystal from breaking other programs that are
attached to the same TTY when it sets O_NONBLOCK on its std filehandles,
and also prevents crystal from breaking itself when spawning a new process.
Two important things:

This changes the std handle numbers. Anyone expecting their programs STDIN/OUT/ERR to be 0,1, and 2, will be (sorta) wrong. This shouldn't actually affect anyone? But if it does, we can save the original FDs and swap em around.
There are two places where I am not dealing with potential error codes and would like to know how I'm supposed to handle that, especially if the output might not be available?

To test, I put these two files in the build directory, and ran tester.sh with bash
tester.sh
#!/usr/bin/env bash

./.build/crystal test.cr

read -p "Prompt: " result
printf '%s\n' "$result"

test.cr
state = STDOUT.blocking
puts "Do we block now? #{state}"

system("echo call a subprocess")

if STDOUT.blocking == state
  puts "We cleanly spawned =)"
else
  puts "ERRAR! Child messed with handles"
end

This checks that the child process doesn't mess with the handle, and also ensures crystal doesn't mess with the TTY's stdin.

The file(s) that define StdFileDescriptor are missing.

Yeah, it's a tad confusing.
El vie., 10 de ago. de 2018 11:23, Jarrod Funnell<EMAIL_ADDRESS>escribió:

@Timbus commented on this pull request.
In src/io/std_file_descriptor.cr
https://github.com/crystal-lang/crystal/pull/6518#discussion_r209276229:

@@ -0,0 +1,26 @@
+require "./syscall"
+require "./file_descriptor"

+class IO::StdFileDescriptor < IO::FileDescriptor

def initialize(@fd : Int32)
@closed = false

If we have a TTY for stdin/out/err, it is a shared terminal.

We need to reopen it to use O_NONBLOCK without causing other programs to break

if tty?

 # Figure out the terminal TTY name

 path = UInt8[256]

Thanks, I misinterpreted this quote from the gitbook: "Static arrays
(StaticArray(Int32, 8), which can also be written as Int32[8])". I get it
now.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/crystal-lang/crystal/pull/6518#discussion_r209276229,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAMx2wVBcJOvrT3purAy7-4B8IIzOoTUks5uPZd1gaJpZM4V4P-R
.

Well its 1am, I'll pick this back up when I wake up.
Ah, for reference this was supposed to be an easier alternative to #6068 but .. we will see.

I hope it works! It's really much simpler.

'Kay. I think it's good now.
It's outside the scope of the original issue, but I might look into handling closed descriptors better. I nearly managed to handle closed FDs:
crystal eval 'puts "bla"' >&-
But handling descriptors being closed dynamically would be more ideal, and would also solve the issue in my last comment.

That's correct. tap must be used.

libuv dup3's the FD to replace the old one with the newly open one.
I'm not sure if that's required here, since the standard file descriptors remain open and referring to the same device.

I initially did that, but then you also need to dup a copy of the original FD so you can pass it to the kid process and it was a bit messy. From what I see libuv always uses new pipes instead of sharing FDs with a child process, so it can afford to dup over the originals (a safer approach, imo)

@Timbus why do the child processes need the original FD? Surely the only thing required is to reset O_NONBLOCK before calling exec?

That would be to solve the issue observed in https://github.com/crystal-lang/crystal/issues/5830 .. Blocking IO on the shared FD halts the event loop. So we have to give the child the original FD and keep ours as nonblocking.

ohhh yeah, my bad, thanks for the correction!


ip_configuration requires primary field

The latest version of the virtual machine scale set resource requires the primary setting within the ip_configuration.  The current main.tf has this:
`
network_profile {
name    = "${var.network_profile}"
primary = true
ip_configuration {
  name                                   = "IPConfiguration"
  subnet_id                              = "${var.vnet_subnet_id}"
  load_balancer_backend_address_pool_ids = ["${var.load_balancer_backend_address_pool_ids}"]
}

}
`
And that fails on plan with the error:
Error: module.computegroup.azurerm_virtual_machine_scale_set.vm-linux: "network_profile.0.ip_configuration.0.primary": required field is not set
If I change the following to this:
`
network_profile {
name    = "${var.network_profile}"
primary = true
ip_configuration {
  name                                   = "IPConfiguration"
  subnet_id                              = "${var.vnet_subnet_id}"
  primary                                 = true
  load_balancer_backend_address_pool_ids = ["${var.load_balancer_backend_address_pool_ids}"]
}

}
`
The plan runs without error.

same issue here
Error: Missing required argument

  on .terraform/modules/computegroup/Azure-terraform-azurerm-computegroup-354b05c/main.tf line 71, in resource "azurerm_virtual_machine_scale_set" "vm-linux":
  71:     ip_configuration {

The argument "primary" is required, but no definition was found.


Make everything in common optional

Like Java, postgres  or tomcat.

I think it's good as-is. Closing.


crate.client.exceptions.ProgrammingError: Invalid server response of content-type 'unknown'

CrateDB version:
1.0.2
JVM version:
1.8.0u111
OS version / environment description:
Amazon Linux, Kernel 3.3
Problem description:
If I changed http port 4300 or transport port (4200) to different port then this exception is thrown
crate.client.exceptions.ProgrammingError: Invalid server response of content-type 'unknown'
Restore to the default port (4200/4300) then there is no issue.

Steps to reproduce:

Use case:
Feature description:

Hi, can you elaborate a bit on how you produce that error?

If I changed http port 4300 or transport port (4200)

This seems mixed up.
Http defaults to 4200
Transport defaults to 4300

Closing this, if this is still a problem feel free to re-open the issue


Expose bug with response codes

This is related to the 204 no content changes made #1532 and #1549 #1550
Seems like the behavior should be as follows:
1. If status was not set: use that.
2. If status was not set:
    a) If body is formatted to an empty string, use 204 No Content
    b) If body is formatted to a non empty string, use a default status for method, (GET: 200, POST: 201, PATCH: 200, DELETE: 200).

I ran into some issues trying to fix at the status method in inside_route.rb:
      def status(status = nil)
        case status
        when Symbol
          raise ArgumentError, "Status code :#{status} is invalid." unless Rack::Utils::SYMBOL_TO_STATUS_CODE.keys.include?(status)
          @status = Rack::Utils.status_code(status)
        when Integer
          @status = status
        when nil
          return @status if @status
          case request.request_method.to_s.upcase
          when Grape::Http::Headers::POST
            201
          when Grape::Http::Headers::DELETE
            if @body.present?
              200
            else
              204
            end
          else
            200
          end
        else
          raise ArgumentError, 'Status code must be Integer or Symbol.'
        end

@body isn't as the return value of the endpoint block
If @body is {} or [] it'll still evaluate as not present, even though it'll serialize to json as a non empty string.

So I think the logic has to be moved til after the formatters run to know if the status should be set to 204 or not.

Coverage decreased (-0.008%) to 99.063% when pulling ede533eca8ac5b09c3c04b887318f927b62d89c6 on remind101:expose-response-code-bug into e1a14bf66ffee7fc0469eca11f56c553c2dfc993 on ruby-grape:master.


x/build: freebsd-arm-paulzhol builder not receiving any work

Reverse pool summary:
host-freebsd-arm-paulzhol: 0/1
Reverse pool machine detail
a20.home.idea-y.com (<IP_ADDRESS>:54566) version 15, host-freebsd-arm-paulzhol: connected 6h13m59.5s, idle for 359.5ms
My logs don't indicate anything out of the ordinary:
====================
2017/08/10 22:36:45 buildlet starting.
2017/08/10 22:36:45 Not on GCE; not remounting root filesystem.
2017/08/10 22:36:45 Dialing coordinator farmer.golang.org:443 ...
2017/08/10 22:36:45 Doing TLS handshake with coordinator (verifying hostname "farmer.golang.org")...
2017/08/10 22:36:46 Registering reverse mode with coordinator...
2017/08/10 22:36:46 Connected to coordinator; reverse dialing active
2017/08/11 03:17:12 buildlet reverse mode exiting.
====================
2017/08/11 03:17:59 buildlet starting.
2017/08/11 03:17:59 Not on GCE; not remounting root filesystem.
2017/08/11 03:17:59 Dialing coordinator farmer.golang.org:443 ...
2017/08/11 03:17:59 Doing TLS handshake with coordinator (verifying hostname "farmer.golang.org")...
2017/08/11 03:18:00 Registering reverse mode with coordinator...
2017/08/11 03:18:00 Connected to coordinator; reverse dialing active

CC @adams-sarah @zombiezen

Probably because of this? https://github.com/golang/build/blob/master/cmd/coordinator/coordinator.go#L1181-L1190

It was supposed to skip any subrepos which are not sys or net.

I see the problem though, it's being called also for the main "go" repo, can I send a CL?

yep, sg.

hm.. not sure why gopherbot is not linking https://golang.org/cl/55094

@paulzhol are you able to merge code and/or deploy coordinator?

@adams-sarah I don't think I have any commit rights.

going to wait to deploy this until the trybot runs clear up. 102 active ATM.


review on indexer for polkadot xcm review

xcm doc:
https://www.notion.so/web3go/XCM-Relay-chain-related-event-extrinsic-bdb19655ff094b0781ec0294c629548c

[ ] code finished
[ ] code reviewed
[ ] running , estimation

estimate min time is 4 days, max is 12 days.

code reviewed.
the data is OK.


ignoringFieldsOnlyLhsHas for usingRecursiveComparison

Feature summary
A function that automatically ignores any fields that only the lhs has when comparing with recursiveComparision. Related to #2143, but for actual POJOs.
Example
record A(String hello, int world){}
record B(String hello, int planet){}
record C( String hello) {}

var a = A("hello", 1);
assertThat(a).usingRecursiveComparison().ignoringFieldsOnlyLhsHas(B("hello", 1)); // complain about a not having `planet`

assertThat(a).usingRecursiveComparison().ignoringFieldsOnlyLhsHas(C("hello")); // this is correct (`world` is ignored)

assertThat(a).usingRecursiveComparison().ignoringFieldsOnlyLhsHas(C("not hello")); // this fails because `"hello" != "not hello"`

I'd also be fine with a function that is just ignoringUnknownFields and doesn't make a difference  between the lhs and rhs.


PWA Service Worker

Unable to register service worker to create an installable application for offline access.

Added service-worker.js to "src" folder.


fix gitDir on windows

git detection didn't take file extension into consideration which caused an error when using git task on windows
fix #117

Thank you for submitting a fix!


refactor table for more DRY styling

add css classes instead of formating every row/column with attributes

done.


import interface with absolute path

I got the following error in my project (using TypeScript 4.6.3):
node_modules/codemirror-graphql/variables/hint.d.ts:3:24 - error TS2307: Cannot find module 'src/hint' or its corresponding type declarations.

3 import { IHints } from 'src/hint';
                         ~~~~~~~~~~

I traced it back to this import statement:
https://github.com/graphql/graphiql/blob/main/packages/codemirror-graphql/src/variables/hint.ts#L22
import { IHints } from 'src/hint';

My fix would be a relative import:
import { IHints } from '../hint';

does<EMAIL_ADDRESS>solve your issue @JaegerJens ?

<EMAIL_ADDRESS>fixes my problem. Thank you!


"View all transactions" shows underneath the help grow glo component

When the transactions are collapsed, they render as follows on Chrome Mac

This is still happening on both testnet.glodollar.org and the deployment branch in the linked PR @Syncretik

ah...that's because I forgot to push those changes up 🤒


Removal of Deployment or Build not possible

Describe the bug
Removal of Deployment or Build via karavan UI is only possible after restart of karavan pod
Steps to reproduce the behavior

Click Build project -> Project is being built and deployed
Go to Tab Deployment -> Click cancel button next to Replicas: 1/1 -> Confirm Deletion -> Popup confirms: Deployment deleted --> Deployment still exists
Go to Tab Build -> Click Delete Build -> Confirm Deletion -> Popup confirms: -> Build pod still exists
Restart karavan pod
Repeat steps 2 and 3 -> Deployment and Build pod is removed immediately

Variant
Web Application
Container Management (if applicable)
Kubernetes
Operating System (if applicable)
Linux
Version
4.8.0
Relevant log output
2024-11-25 10:57:06,922 INFO  [org.apa.cam.kar.kub.KubernetesService] (executor-thread-6) Delete deployment: demo-energy in the namespace: karavan
2024-11-25 10:57:06,959 ERROR [org.apa.cam.kar.kub.KubernetesService] (executor-thread-6) Failure executing: DELETE at: https://<IP_ADDRESS>:443/apis/apps/v1/namespaces/karavan/deployments/demo-energy. Message: Unauthorized. Received status: Status(apiVersion=v1, code=401, details=null, kind=Status, message=Unauthorized, metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=Unauthorized, status=Failure, additionalProperties={}).

After restart of karavan pod
2024-11-25 11:47:33,336 INFO  [org.apa.cam.kar.kub.KubernetesService] (executor-thread-4) Delete deployment: demo-energy in the namespace: karavan
2024-11-25 11:47:33,363 INFO  [org.apa.cam.kar.kub.DeploymentEventHandler] (-671536858-pool-5-thread-1) onDelete demo-energy
2024-11-25 11:47:33,384 INFO  [org.apa.cam.kar.kub.ServiceEventHandler] (-671536858-pool-5-thread-2) onDelete demo-energy
2024-11-25 11:47:33,411 INFO  [org.apa.cam.kar.kub.PodEventHandler] (-671536858-pool-5-thread-1) onUpdate demo-energy-d7ffc4c57-7wklr
2024-11-25 11:47:33,440 INFO  [org.apa.cam.kar.kub.PodEventHandler] (-671536858-pool-5-thread-2) onUpdate demo-energy-d7ffc4c57-7wklr

ServiceAccount configured for Karavan deployment should have permissions to delete deployments, service, pods, etc in karavan namespace


Handling invalid picture format

Someone uploaded a picture in .webp format (https://developers.google.com/speed/webp/) which is currently not valid.
Either we have to catch the error or to make the use of this format possible.

https://stackoverflow.com/questions/52176142/error-with-webp-images-when-using-active-storage-on-heroku

@netagonen and I tried to reproduce this error but uploading a webp file worked without any problems.
But we found out that we could validate image type with this code:
  validate :image_type
def image_type
    if !image.content_type.in?(%('image/jpeg image/png'))
        errors.add(:image, "needs to be a jpeg or png!")
    end
end

For now I excluded use of other formats than jpg, png or gif in this PR #1085. I would recommed to close this issue to get an better overview what is really needed. What do you mean @zaziemo ?


Unable to start rebar3 compiled script

From a Windows 7 machine, with Erlang OTP 17 installed, through an MSYS2 sub-system, I'm unable to run the rebar3 compiled script. The error is the following:
$ rebar3
escript: exception error: undefined function rebar:main/1
  in function  escript:run/2 (escript.erl, line 752)
  in call from escript:start/1 (escript.erl, line 276)
  in call from init:start_it/1 (init.erl, line 1057)
  in call from init:start_em/1 (init.erl, line 1037)

The sha1 of the script I downloaded from https://s3.amazonaws.com/rebar3/rebar3 is 133cc539f675a10e7fa1745470aeb0f006c527ce. Note that rebar 2.x compiled script works on the same system.
Tried also to compile from source but failed because of a small problem between the MSYS2 sub-system and the pure Windows Erlang tooling installed. But tha'ts another story. I might workaround it, will try tomorrow if I can find some time.
Regards,
Matt

For windows, I believe we need to run a special build option from the source file by calling ./bootstrap/bootstrap -- which should dump a rebar.cmd file. I'm not sure if renaming it to rebar3 will work. Things should be compatible, but neither of the contributors have Windows at hand right now.

Hi Fred,
Thanks for you response. However, I must emphasis that the original compiled rebar script works as-is, without a Windows wrapper inside a .cmd file. MSYS2 (derived from Cygwin) is POSIX emulation layer capable of interpreting the compiled rebar script.
I just tested on my work machine and I was able to run the rebar3 script there. The system is similar, main difference is that I have Erlang R15B03 installed instead of 17. This might be the culprit. I will install OTP Erlang 17 and see how it goes.

I just tested with Erlang OTP 17 on my work machine and it works correctly.
I noticed also that the current version sha1 is different than the one I downloaded yesterday.

Yesterday - 133cc539f675a10e7fa1745470aeb0f006c527ce
Just now - 60a0b82a6eee91ccbe960548680310e833a47ca0

Maybe it was simply a packaging issue. I will re-download when I get home this evening and close the issue if it works with the new version.
Regards,
Matt

It's equally possible some function we use has a backwards-incompatibility and OTP 17 fixes that, but it's hard to know without having a work machine to test it with. Tuncer sent me a link to a Travis equivalent that does Windows, but I've been unwilling to add them to my account given there's work-related stuff in there it wanted access to.

Ok finally, I found something. Went home, installed Erlang 15, 16 and I had already 17 installed. Tried the alpha-1 version (60a0b82a6eee91ccbe960548680310e833a47ca0), all versions worked correctly.
Re-put in place the old script I had downloaded yesterday (133cc539f675a10e7fa1745470aeb0f006c527ce), tried with all three Erlang installations, and magic, it now works!
So, I was wondering why it was not working yesterday. It's at this point that I realized that yesterday, I had rename the downloaded script from rebar3 to simply rebar. Now, the old and the new versions are crashing with the same message about not being able to find out rebar:main/1 method.
Just to check if it's only a Windows problem, I pop a CentOS 6.4 VM, downloaded erlang, downloaded rebar3, renamed to rebar and ran it. Same problems occurred. So it seems renaming the script to something else than rebar3 make it crash somehow.
It's probably something expected for escript has I just tried with rebar 2.x compiled script and it exhibits the same behavior. A bit unfortunate in my opinion, but not big deal.
Adding an entry in the FAQ or elsewhere about this problem could help others like me having this problem.
I'm closing this issue, thanks for your support.


Keepalive false alert because of time jump in logs

Hello
when we restart a VM with sensu-client we getting critical alerts (configured after 180s) even when the machine reboot takes less that a minute. In the sensu-client logs we can see that the time jumps 7 hours which we believe its related to that false alert. Also to mention that the servers time its set to UTC+7. The time of the server is properly set and ntpd is running, so we can rule out actually time issues on the server itself.
In uchiwa we can also see that the error code imidiatly jumps from 0 -> 2. The warning which is a 1 is not even shown.

When restarting just the sensu service we dont see this behavior.
{"timestamp":"2018-12-10T19:07:15.364580+0700","level":"warn","message":"received signal","signal":"TERM"} {"timestamp":"2018-12-10T19:07:15.364736+0700","level":"warn","message":"stopping"} {"timestamp":"2018-12-10T19:07:15.365169+0700","level":"info","message":"completing checks in progress","checks_in_progress":[]} {"timestamp":"2018-12-10T19:07:15.866074+0700","level":"info","message":"closing client sockets"} {"timestamp":"2018-12-10T19:07:15.874649+0700","level":"warn","message":"stopping reactor"} {"timestamp":"2018-12-10T12:09:03.391646+0700","level":"warn","message":"config file does not exist or is not readable","file":"/etc/sensu/config.json"} {"timestamp":"2018-12-10T12:09:03.393511+0700","level":"warn","message":"ignoring config file","file":"/etc/sensu/config.json"} {"timestamp":"2018-12-10T12:09:03.393562+0700","level":"warn","message":"loading config files from directory","directory":"/etc/sensu/conf.d"} {"timestamp":"2018-12-10T12:09:03.393603+0700","level":"warn","message":"loading config file","file":"/etc/sensu/conf.d/rabbitmq.json"} {"timestamp":"2018-12-10T12:09:03.393635+0700","level":"warn","message":"loading config file","file":"/etc/sensu/conf.d/client.json"} {"timestamp":"2018-12-10T12:09:03.393688+0700","level":"warn","message":"config file applied changes","file":"/etc/sensu/conf.d/client.json","changes":{"client":{"name":["as-propp-3b49.agprod2.agoda.local","as-propp-3b49"],"keepalive":[null,{"handlers":["default","keepalive"],"thresholds":{"critical":180,"warning":120}}],"subscriptions":[null,["linuxvm","adp_forwarder_2.4.1","as-propp-3b49"]]}}} {"timestamp":"2018-12-10T12:09:03.393729+0700","level":"warn","message":"loading config file","file":"/etc/sensu/conf.d/adp_uforwarder.json"} {"timestamp":"2018-12-10T12:09:03.393761+0700","level":"warn","message":"config file applied changes","file":"/etc/sensu/conf.d/adp_uforwarder.json","changes":{}} {"timestamp":"2018-12-10T12:09:03.393793+0700","level":"warn","message":"loading config file","file":"/etc/sensu/conf.d/devops_consul-client.json"} {"timestamp":"2018-12-10T12:09:03.393824+0700","level":"warn","message":"config file applied changes","file":"/etc/sensu/conf.d/devops_consul-client.json","changes":{}} {"timestamp":"2018-12-10T12:09:03.393863+0700","level":"warn","message":"applied sensu client overrides","client":{"name":"xxx","address":"<IP_ADDRESS>","keepalive":{"handlers":["default","keepalive"],"thresholds":{"critical":180,"warning":120}},"subscriptions":["linuxvm","xxxx","as-propp-3b49","consul-client-linux","client:xxx"]}} {"timestamp":"2018-12-10T12:09:03.399812+0700","level":"warn","message":"loading extension files from directory","directory":"/etc/sensu/extensions"} {"timestamp":"2018-12-10T12:09:03.399888+0700","level":"info","message":"configuring sensu spawn","settings":{"limit":12}} {"timestamp":"2018-12-10T12:09:10.280473+0700","level":"info","message":"received check request","check":{"command":"/opt/sensu/embedded/bin/ruby /etc/sensu/plugins/http/check-http.rb -u http://localhost:8500/v1/health/node/$(hostname) --noproxy --query 'Agent alive and reachable' --response-code 200","handle":true,"handlers":["logstash","opsgenie"],"standalone":false,"refresh":900,"occurrences":3,"dependencies":["keepalive"],"tags":["it-devops-configuration","consul"],"owner":"it-devops-configuration","service":"consul","severity":"high","aggregate":false,"addInfo":"Please slack it-devops consul team: #devops_consul_monitor","name":"consul-sevice-check-linux","issued":1544443750}} {"timestamp":"2018-12-10T12:09:10.505206+0700","level":"info","message":"publishing check result","payload":{"client":"xxx","check":{"command":"/opt/sensu/embedded/bin/ruby /etc/sensu/plugins/http/check-http.rb -u http://localhost:8500/v1/health/node/$(hostname) --noproxy --query 'Agent alive and reachable' --response-code 200","handle":true,"handlers":["logstash","opsgenie"],"standalone":false,"refresh":900,"occurrences":3,"dependencies":["keepalive"],"tags":["it-devops-configuration","consul"],"owner":"it-devops-configuration","service":"consul","severity":"high","aggregate":false,"addInfo":"Please slack it-devops consul team: #devops_consul_monitor","name":"consul-sevice-check-linux","issued":1544443750,"executed":1544418550,"duration":0.224,"output":"CheckHttp OK: 200, found /Agent alive and reachable/ in 259 bytes\n","status":0}}} {"timestamp":"2018-12-10T19:09:17.575396+0700","level":"info","message":"received check request","check":{"handle":true,"standalone":false,"refresh":900,"dependencies":["keepalive"],"command":"/etc/sensu/plugins/linux/check-procs.rb --pattern 'nscd' --critical-under 1 --critical-over 1","occurrences":3,"service":"linux_basic_checks","owner":"it-infra-external","severity":"high","aggregate":false,"name":"linux-nscd-process","issued":1544443757}}
Your Environment

Sensu version used: 1.6.1
Active Sensu extensions (e.g. WizardVan 0.1.0): None
Operating System and version (e.g. Ubuntu 14.04): CentOS7
Transport and runtime versions (e.g. RabbitMQ 3.6.2 on Erlang 18.2):

Quick update, we tried some different versions on the client but the issue persist.

Client: 1.6.1 & Server: 1.6.1
Client: 1.0.2 & Server: 1.6.1


Questions about metrics

Hi! Thanks for your wonderful work.
I want to ask a question about the metrics. I read your code in test_generation.py and find that the metrics are calculated between generated sample and test sample read from dataset.  In your diffusion model,  generated samples are random generated and it may differ in each  running result. So when calculating the Chamfer Distance( calculate the distance between generated samples and test samples), it may cause a doubt result. I want to know how to guarantee a reliable result.
Best.

@yufeng9819 Hi, do you know which metrics correspond to those in Tabel 1? '1-NN-CD-acc' and '1-NN-EMD-acc'?


Auto-generate documentation from JSDoc

https://github.com/andstor/jsdoc-action
https://www.gitbook.com/
https://medium.com/@kevinast/integrate-gitbook-jsdoc-974be8df6fb3

Project icon: https://www.flaticon.com/free-icon/git_4908200?term=git&page=1&position=17&page=1&position=17&related_id=4908200&origin=search

We may need to do extra work to support Typescript with JSDoc
https://www.npmjs.com/package/jsdoc-plugin-typescript

TypeDoc themes
https://github.com/google/typedoc-neo-theme

TypeDoc GitHub Actions deployment
https://github.com/TypeStrong/typedoc/issues/1485
https://github.com/TypeStrong/typedoc-action

Try uploading to GitHub Wiki: https://github.com/tgreyuk/typedoc-plugin-markdown/issues/118


Fedora/Nobara 40 depency issue

System Configuration:
Default Nobara 40 full upgraded
Steps to recreate:
Download password fix release
execute in terminal
> ./FOLON-Downgrader
curl: /tmp/_MEIfWxnoP/libssl.so.3: version `OPENSSL_3.2.0' not found (required by /lib64/libcurl.so.4)
Traceback (most recent call last):
  File "FOLON-Downgrader.py", line 926, in <module>
  File "FOLON-Downgrader.py", line 874, in main
  File "FOLON-Downgrader.py", line 41, in SetupFont
  File "zipfile.py", line 1253, in __init__
FileNotFoundError: [Errno 2] No such file or directory: 'FOLON-Downgrader-Files/overseer.zip'
[10396] Failed to execute script 'FOLON-Downgrader' due to unhandled exception!

DNF shows
> sudo dnf install openssl
[sudo] password for XXXXXXXX:
Last metadata expiration check: 0:02:13 ago on Wed 24 Jul 2024 04:13:46 PM CDT.
Package openssl-1:3.2.1-2.fc40.x86_64 is already installed.
Dependencies resolved.
Nothing to do.
Complete!

could you check the result of curl -v?

If you check in the subfolder FOLON-Downgrader-Files does a Fonts folder exist?

Negative

does
curl -sqL "https://dl.dafont.com/dl/?f=overseer" -o FOLON-Downgrader-Files/overseer.zip

work in that directory?

It does

and a overseer.zip file's oresent?

Correct.
When running the release again after the curl command downloading overseer.zip
> ./FOLON-Downgrader
curl: /tmp/_MEIzn6UWV/libssl.so.3: version `OPENSSL_3.2.0' not found (required by /lib64/libcurl.so.4)
xdg_toplevel@33: error 2: minimum width can't be bigger than the maximum width
The Wayland connection experienced a fatal error: Protocol error

I've made a new version that might fix this, please remove the folder and try running this

This might fix it but I have no idea https://github.com/Fallout-London/FOLON-FO4Downgrader/releases/tag/1.4.3

Curl isssue resolved. Opening Wayland issue


"Do-It" Button on Work Items in Detail View [2]

Description
As a user, I want to open a Che context for a Work Item from the Work Item detail view.
Functional Acceptance Criteria

A Work Item includes fields with meta information used for linking to a Che context.
The new fields are internal only and are not displayed or being editable by the user.
If the fields are present (non empty), the Work Item detail view shows a link or button.
Clicking on the button opens Che in a new browser window using an URL constructed from the meta fields on the Work Item.

Non-functional Acceptance Criteria
None.
Note: The exact nature of the extra field is to be defined. Also the way of creating the URL from the field values.
Note: This story currently only describes linking to Che. For other linkings, there will be a new architecture being developed later that is more flexible. The implementation done for this story can then be regarded legacy.
Note: The URL must be constructable only from the meta fields on the Work Item and static contents hardcoded in the Planner.

PR from core tells you how to get editor's URL, it's value is not yet finalised.
Note: Do not consume the value of URL to perform any testing.

As per conversation in PR, UI team need to use relationships->codebase->links->meta["edit"] attribute to locate the do-it-URL. If this attribute is set then enable the do it button else not.
Core will not be giving value for this attribute because other modules are not yet ready to integrate that piece and strategies for the same are not yet finalised. Hence no code changes from core for this particular task.


Fix bad indent in docs

Closing as there are far too many docs warnings (treated as errors) to warrant fixing every single one of them for an update


Add support for structured nets (IUS Real Number  Models).

This allows CocoTB to observe structured nets, like the EEnets that are used in IUS AMS/Real Number Model flows. You can't drive individual components (even in SystemVerilog), so the solution for driving EEnet types would be similar to the following:
import EE_pkg::*;

module dut(inout EEnet out);
    real out_v, out_r;

    // EEnet = '{V, I, R};
    assign out = '{out_v, 0, out_r};
endmodule

You can then drive out_v and out_r from CocoTB. This change will allow you to also observe values via dut.out.V.value, for example.
I also made a small change to allow float convertible types (like int) to be assigned to real values.

also made a small change to allow float convertible types (like int) to be assigned to real values.

Would be nice to add a test for this.

@nevsan do you think you'll have a chance to incorporate the remaining review comments into this to get this PR merged? Unfortunately I cannot really test on IUS and cannot follow up on it.

No feedback unfortunately, closing this until someone can pick it up.


[entropy_src/rtl] return ack fsm to idle when disabled

In the case the entropy_src is disabled and a request from CSRNG is pending, the state machine should return to idle and wait to be enabled again.
Fixes #10133.
Signed-off-by: Mark Branstad<EMAIL_ADDRESS>

Duplicate of #10146 . Closing this one.


Can't resolve fakecss-path with splitting: true

One more time ))) I just want explain situation:
I have splitting bundles with css-chunks, but i can't load css on page...

index.js

index.css

I have some resolver for this, but its not working with splitting: true...
let fakecsspathResolvePlugin = {
    name: 'fakecsspath',
    setup(build) {
        let path = require('path')

        build.onResolve({ filter: /\.esbuild-svelte-fake-css$/ }, ({ path }) => {
            return { path, namespace: 'fakecss' }
        })

        build.onLoad({ filter: /\.esbuild-svelte-fake-css$/, namespace: 'fakecss' }, ({ path }) => {
            const css = cssCode.get(path);
            return css ? { contents: css, loader: "css" } : null;
        })
    },
}

may be need use namespace: 'file' ?

Certainly something to try. I currently use the fakecss namespace because the css files don't exist on the actual file system, the plugin creates them on a virtual filesystem. From the esbuild documentation:

By default esbuild operates in the file namespace, which corresponds to files on the file system. But esbuild can also handle "virtual" modules that don't have a corresponding location on the file system.
...
Virtual modules usually use a namespace other than file to distinguish them from file system modules.

In this case, I consider the fake css to be a virtual module and thus use a different namespace. Can you please clarify where Evan recommends the file namespace?
Additionally can you please clarify what you mean when you say

when //fake-css includes links in single css/js file it's work ok...

Something you could try would be to replace the fakecss namespace in the dist/index.js / dist/index.mjs files of this plugin with file and see if that makes a difference. If it does, report back and we can talk about what that looks like.
I'm also confused why there is a need for the fakecsspathResolvePlugin.
esbuild does not support css splitting at the moment and that isn't something that I want to attempt to solve with this plugin at the moment. You could disable css file output from Svelte which isn't ideal but might go a long way towards solving this for now.

Certainly something to try. I currently use the fakecss namespace because the css files don't exist on the actual file system, the plugin creates them on a virtual filesystem. From the esbuild documentation:

By default esbuild operates in the file namespace, which corresponds to files on the file system. But esbuild can also handle "virtual" modules that don't have a corresponding location on the file system.
...
Virtual modules usually use a namespace other than file to distinguish them from file system modules.

In this case, I consider the fake css to be a virtual module and thus use a different namespace. Can you please clarify where Evan recommends the file namespace?

https://github.com/evanw/esbuild/issues/808#issuecomment-806087899

Additionally can you please clarify what you mean when you say

when //fake-css includes links in single css/js file it's work ok...

Something you could try would be to replace the fakecss namespace in the dist/index.js / dist/index.mjs files of this plugin with file and see if that makes a difference. If it does, report back and we can talk about what that looks like.
I'm also confused why there is a need for the fakecsspathResolvePlugin.
esbuild does not support css splitting at the moment and that isn't something that I want to attempt to solve with this plugin. You could disable css file output from Svelte which isn't ideal but might go a long way towards solving this for now.

https://github.com/evanw/esbuild/issues/808#issuecomment-806087899
has no effect

ah! I just realized that you are wanting to automatically import the generated css files from js! You gave the problems you were having but not the expected outcome
esbuild has a few breaking changes that are about to be released that should make this easier. Pretty sure this is now a feature request given that importing the css files from js isn't something this plugin currently supports. If that isn't correct please feel free to correct me!

Today new release 0.12.0 fix it 👍🏻
I’m m happy 🤓

Awesome glad to hear it!


adoc table generator misses to export go slice []string for types

We have done some improvements to the adoc table generator for the documentation, but one item does not get exported. This is when it comes to exporting the type in particular []string. You can see that nothing gets exported when looking into thumbnails_configvars.adoc at field THUMBNAILS_RESOLUTIONS (you must use raw to see the generated not rendered content). The two plus symbols are necessary and added by the adoc template to tell antora to pass the value inbetween. The value would be correct when looking like: +[]string+
@dragonchaser

Since this is working as expected in https://github.com/owncloud/ocis/blob/master/docs/helpers/environment-variable-docs-generator.go.tmpl I would assume this is a bug in https://github.com/owncloud/ocis/blob/master/docs/helpers/adoc-generator.go.tmpl


Investigate live trace runs

Doing in process traces using ts lib would allow us to provide more information in an on-demand manner.  For example, types.json only includes the display field when it is cheap to generate.  With a live trace we could generate the displays as needed.
As a side benefit, in memory fs should reduce skew due to IO overhead during tracing.

partially implemented with the ability to run a command to do traces and automatically send them to the viewer.
live via ts lib still should be investigated but is low priority

this is higher priority now as it enables much more functionality.  It can't be done in process without blocking the event loop and freezing the vscode UI, so I'm working on a server we can spawn and communicate with via web sockets


Blue tiles around town river

remove them

It would be good to find out the purpose of those tiles

These tiles are described as "Weird" here: http://paul.siramy.free.fr/_divers/ds1/doc/


Update sublime-text to 3170

After making all changes to the cask:

[x] brew cask audit --download {{cask_file}} is error-free.
[x] brew cask style --fix {{cask_file}} left no offenses.
[x] The commit message includes the cask’s name and version.

Thank you, but I saw #46773 first.


Coping a page to the same Tree will explode recursively

Issue Summary
If you try to copy a Page which has childs and use the destination the same tree "branch" it will explode when trying to recursively copy.
Steps to Reproduce

Access Wagtaildemo
Copy Event listing adding the destination as itself
treebeard.exceptions.PathOverflow: The new node is too deep in the tree, try increasing the path.max_length property and UPDATE your database

Technical details

Python version: Python 3.4.3
Django version: Django (1.10.4)
Wagtail version: 1.9a0.
Browser version: http://whatsmybrowser.org/b/EJM4TK7

The first commit just add a test which will fail to show the bug I'm relating.
The second one adds a possible solution to this problem. I'm just raising an Exception for now, how do you want to handle this particular case? I think it is ok to raise an exception in this specific case because Wagtail UI should not let the user get to this state.
We still need to improve UI to show the node being copied and it's children as disabled.
What is your opinion about this? Do you want me to create a PR?

Thanks @x-warrior!
For the 'move page' operation, we dealt with this by adding a permission rule that prevents all users (including superusers) from moving a page into one of its descendants:
https://github.com/wagtail/wagtail/blob/5e13a1d6745a77d558976c33670d9bb79fe1f703/wagtail/wagtailcore/models.py#L1784-L1786
I guess we're missing a similar rule for the 'copy page' operation.

@gasman I have added a few changes and I think now it is more aligned to the solution we have in Wagtail.
I'm keeping the verification inside the copy method to prevent this scenario if someone try to call copy directly from a different place. (Since the verification is made on the view).
What do you think? Can I create a PR?
We will need some UI improvements, because right now the "ModelChooser" does let you select any type of model and in this particular case we want to exclude specific pages when copying subpages.

What do you think? Can I create a PR?

Yes please!


Support multiple num fewshot

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.0 out of 2 committers have signed the CLA.:x: leemengtw:x: mkshingYou have signed the CLA already but the status is still pending? Let us recheck it.


Support slice nesting like < a = x[ y[ z ] ] >

https://github.com/pikasTech/pikascript/commit/9256a872584c6d6543cd47422a902786be39951a


Update handling of registryUrls to match expectation from registry.devfile.io

What does this PR do?
Update the flatten library to resolve plugins/parents from <registryUrl>/devfiles/<id>, as this appears to be what's expected in the main devfile registry -- e.g. https://registry.devfile.io/viewer/devfiles/Community+nodejs-basic uses
parent:
  id: nodejs
  registryUrl: "https://registry.devfile.io"

which refers to the devfile https://registry.devfile.io/devfiles/nodejs
What issues does this PR fix or reference?
N/A
Is it tested? How?
Test cases are updated.
PR Checklist

[ ] E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)

[ ] v8-devworkspace-operator-e2e: DevWorkspace e2e test
[ ] v8-che-happy-path: Happy path for verification integration with Che

/test v8-devworkspace-operator-e2e


Add Faker::PhoneNumber.country_code

Fixes #558

@vbrazo Hey, I added required changes as you said.


How do I combine extensions?

I have a custom extension called "words" that can convert a number to text. So if I do {Age:words()} would get something like "twenty-seven" instead of 27. I would like to combine this with another extension I have called "capitalize" so when doing {Age:words():capitalize()} I would get "TWENTY-SEVEN". This does not work, however. Is this possible?

No, this does not work, formatter extensions cannot be linked. In the given example you could add an argument to your words extension, e.g. {Age:words(lc)} or {Age:words(uc)}, though. I'd appreciate if you shared your words extension :)


Add ghost-gcs-backup

Signed-off-by: Maartje Eyskens<EMAIL_ADDRESS>

I'd also add the limit env variable suggested in https://github.com/src-d/infrastructure-dockerfiles/pull/23#discussion_r287325536


Allow command line arguments to have project local persisted defaults

(This is a follow up issue to #3700 with an expanded problem description and alternative solution.)
background
When working in many projects with lots of other people I have seen that I have to who might not know or understand the implications of all the details of how pipenv works,  some people are just programmers and should not have to be bothered by these details.
Pipenv is a fairly complicated tool and for most non maintainer developers of a project being able to set these values in the project repository would be very good from a developer experience perspective.
setting default project local persistent values
I would as an example like that pipenv install were always by default running with --keep-outdated and  maybe --selective-upgrade unless otherwise specified in a lot of projects.
using environment variables
Since the two settings I mentioned (and some other) can be set by environment variables environment variables can be used. However, I see two very significant problems with environment variables:

They do not propagate automatically to every developer of a project/repository which is the whole point of this change proposal.
For a lot of windows developers environment variables are not obvious features at all. (I've spent a considerate amount time over my career helping windows developers set up and understand environment variables in various contexts even though I'm not a Windows user myself)

possible solution
I suggest that pipenv starts trying to read a tool specific env file like pipenv.env before reading the plain .env in the same directory as the Pipfile.
This would allow defaults to be specified in pipenv.env and checked in into the repository while  .env still can be used for local settings override.

As a general comment I think I personally would lean towards putting things in pyproject.toml at this point -- everyone else seems to do that
On the --keep-outdated point I think --keep-outdated is too polluted of a command right now and is super confused.  The naming of it and the behavior of it performs kind of a split function and that needs to be clarified in some real way -- it partly attempts to make a minimum viable change to your lockfile, but only if you are introducing new packages.  On the other hand it tries to make sure not to drop any dependencies that may have been lost due to mismatched environment markers.
Because of that confusion it has a super limited window of utility, but is incredibly useful for that.  pipenv install --selective-upgrade is the actual useful command because it lets you upgrade / install only the minimum required set of packages given a specific requirement.
The missing command is one that will re-lock to the current versions across the board but retain anything with environment markers that would otherwise drop out. That specific point makes me want to break the command apart before doing anything in the way of recommending that people use it
Overall I'd definitely be interested in a PEEP on the topic because I think you're spot on, and once the behavior is defined I don't think it would be that hard to work out


Add new commands for DR pre-requisite

/test test

Ugh, the bot merged it after a single approval even though there were still outstanding requests. I guess we'll need a follow-up PR, thanks.


Could not find a version that satisfies the requirement supabase (from versions: none)

Can't install python client for superbase
A clear and concise description of what the bug is.
Could not find a version that satisfies the requirement superbase (from versions: none)
pip install supabase

@J0

@awalias @J0 @fedden @awalias
Any update on this stuck for 3 days as the pip installation was failing.

Hi @pvbhanuteja if urgent please install locally after cloning develop branch

Will get around to a release via pypi soon

Hi @fedden thanks for that, there is at least some progress in building up my logic.
I got caught with another issue cant update the record opened a new issue
AttributeError: 'RequestBuilder' object has no attribute 'eq'

Hey @pvbhanuteja,
@dreinon and I have just tested and we should now be able to install with pip install supabase we are closing this issue now but let us know if you face any further problems.


MIT license instead of BSD 3-clause

My application automatically merges all files into 1 large minified file that unintentionally eliminates the license notice. And adding this notice is not an easy stuff to me. I really appreciate if you change your license to MIT.

You can preserve license-related comments during minification.
I just added the @license tag for this to work, in v2.1.2
UglifyJS will preserve comments containing this tag if you run it with the --comments option. This should work with Google Closure Compiler too.
BSD-3-clause and MIT are compatible, both can coexist in the same packaged file.


Running Standalone - yarn start not working

Hey Benedikt - stuck at running the script as a standalone solution.
When trying to start "yarn start" it tells me:
janbeco@Mac5k diasend-nightscout-bridge % yarn start
yarn run v1.22.19
error Command "start" not found.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
janbeco@Mac5k diasend-nightscout-bridge %
I also couldn't figure out what I have to rename the .env.example file to. Tried all different options. But when using the command "yarn run run" it tells me:
janbeco@Mac5k diasend-nightscout-bridge % yarn run run
yarn run v1.22.19
$ yarn build && node dist/run.js
$ tsc
/Users/janbeco/Documents/GitHub/diasend-nightscout-bridge/dist/index.js:183
throw Error("Diasend Username not configured");
^
Error: Diasend Username not configured
at startPumpSettingsSynchronization (/Users/janbeco/Documents/GitHub/diasend-nightscout-bridge/dist/index.js:183:15)
at Object. (/Users/janbeco/Documents/GitHub/diasend-nightscout-bridge/dist/run.js:5:41)
at Module._compile (node:internal/modules/cjs/loader:1218:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1272:10)
at Module.load (node:internal/modules/cjs/loader:1081:32)
at Module._load (node:internal/modules/cjs/loader:922:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:82:12)
at node:internal/main/run_main_module:23:47
Node.js v19.3.0
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
It looks like the env variables cant be found.
Appreciate any pointers.
Thanks a lot and GREAT work! Looking forward getting this thing running.
All the best from almost Munich,
.Jx

In the meantime I hardcoded the credentials in config.ts. Can start it now "yarn run run".... But i guess there is a "better" version to do that?! ;)

There are a variety of options of passing environment variables to the command you're running.
The easiest (but also most verbose way) would be to specify them right in front of the yarn run run command like:
<EMAIL_ADDRESS>DIASEND_PASSWORD='my-pasword' yarn run run

Note that this will only work on UNIX-based shells so it will not work on Windows.
As for the .env.example file, the values in there need to be loaded manually or via a solution like direnv.

This info should maybe go in your readme. It starts with editing all the details in the .env.example file but it doesn't say what to do with it afterwards. As well as (at least on macos) the yarn start command is not working....

As for the .env.example file, the values in there need to be loaded manually or via a solution like direnv.

How would the manual way work? I wasn't able to figure out how to define the variables... (on macos)!
Thanks.

In the meantime I hardcoded the credentials in config.ts. Can start it now "yarn run run".... But i guess there is a "better" option to do that?! ;)

PS: is just putting the login credentials in config.ts wrong or would/does that also work ;)

In the meantime I hardcoded the credentials in config.ts. Can start it now "yarn run run".... But i guess there is a "better" option to do that?! ;)

PS: is just putting the login credentials in config.ts wrong or would/does that also work ;)

it surely does work. One downside is that it might break whenever config.ts changes in future versions of the software so that your local modifications are overwritten and - at least as a developer - there's a high risk to accidentally commit the local changes to version control and thereby leak your credentials to the world.

This info should maybe go in your readme. It starts with editing all the details in the .env.example file but it doesn't say what to do with it afterwards. As well as (at least on macos) the yarn start command is not working....

As for the .env.example file, the values in there need to be loaded manually or via a solution like direnv.

How would the manual way work? I wasn't able to figure out how to define the variables... (on macos)! Thanks.

Manually, I'd suggest to prefix each line in the .env file with export , i.e.
export DIASEND_CLIENT_ID="a486o3nvdu88cg0sos4cw8cccc0o0cg.api.diasend.com"
export DIASEND_CLIENT_SECRET="8imoieg4pyos04s44okoooowkogsco4"
export DIASEND_USERNAME="<your-diasend-username>"
export DIASEND_PASSWORD="<your-diasend-password>"
export NIGHTSCOUT_URL="<my-nightscout-url>"
export NIGHTSCOUT_API_SECRET="<nightscout-api-secret>"

You can then just run source .env from within your terminal before running yarn run run and all the variables should be set.
Alternatively, if you don't want to add the export statement, you can try some bash-stuff like eval $(cat .env) yarn run run (untested).
I personally always use direnv as it will automatically load the .env file when I cd into the directory (given that there's a suitable .envrc file like in this repository).


Make dependency version requirements less restrictive

[x] Add .ruby-version to .gitignore, because there's no strict dependency on the specific version of Ruby
[x] Lift up the dependency version restrictions a bit

It would be great if you can release an updated version to Rubygems so that dependant applications can upgrade to stellar-base 0.14.0.
/cc @nullstyle

I feel the .ruby-version is meant mainly for developers of the gem to default to a specific version of Ruby, mainly to avoid confusion. I find this useful.

I do want less resitrictive gem versions though 👍

LGTM.  I'll look into making a release tomorrow.  thanks!

@nullstyle Thanks for merging. Are there any blockers for release?

I just released v0.14.0 of stellar-base and v0.4.0 of stellar-sdk to rubygems @nebolsin.   Sorry about the delay!


Add getter for remote address in QuicChannel

Motivation
For my project, it would be useful to determine the country of connection. Even though the address may change, it would be convenient to know it in any case, for example, when connection is created.
Modification
Add getter to QuicChannel
After that, remote can be obtained as:

((QuicChannel) channel).getRemote()

Result
Fixes #263

will have a look soonish

Is this for the server or client side use-case ?

@normanmaurer
For server side.

@abezhovets got it... I think it would be better to fire this as an event (the same as we do for QuicConnectionMigrationEvent. WDYT ?

@normanmaurer
Please check.
Did I get your idea right?

@abezhovets yes you did...

@abezhovets I wonder if we want to merge this with the concept of QuicConnectionMigrationEvent tho.
Maybe rename QuicConnectionMigrationEvent to QuicConnectionEvent and use the same event for this by using null as  from. WDYT ?

@normanmaurer
At first glance it looks ok. Made minor changes to the QuicChannelValidationHandler. Or maybe delete it altogether?

@abezhovets thanks a lot!

Thanks to all, looking forward to release.


⚠ Bofrost.lu has degraded performance

In 746c026, Bofrost.lu (https://www.bofrost.lu) experienced degraded performance:

HTTP code: 200
Response time: 1445 ms

Resolved: Bofrost.lu performance has improved in acef7f8 after 14 minutes.


Task - Browse Nomenclature - Type specimen

Type specimen is not shown. Including Toggle the type option

Can you confirm: When a type specimen is set, then you want to see the label/text in the type box that brings you to the type form?

We can close for now, not sure what it was after year and a half.


login: Support logging out of an account

Screenshots coming soon. :)
Fixes: #463

This implements the logic where per-account routes on the nav are removed on logout, but it doesn't animate their exits; they just disappear suddenly. I think that's probably fine, and I didn't find a way to add the animation in.

Marked as a draft because in particular I think this may be lacking some test coverage that we'll want; for example, of the various if (maybeAccount == null) return;s added in PerAccountStore.

Thanks for the review! Revision pushed.

Thanks for the review! Revision pushed.

Do the screenshots need to be updated for this one?

Do the screenshots need to be updated for this one?

Ah right! Actually the UI hasn't changed from what's shown in the screenshots in the PR description. I remember us talking in the office about making it look better, but I don't remember if we settled on something different. If there are changes to make, how about doing them as a followup?

In the interest of getting things merged efficiently, maybe let's split this PR up

Sounds good. I just sent #1007 for those first several commits, and I'll send a followup with the remaining ones (implementing logout) as a followup. Then as mentioned in my previous comment, maybe any UI changes can be a followup to that.


File structure cleanup

Everything should be a little clearer now. In addition I have also revised the now LaunchUrlHelper a bit.
Maybe we can even move the cards into widgets, but I left them out for now.

@darrenaustin I added the private constructor, thanks a lot for the review everyone,


[video_player]version 2.2.18 "Moves Android and iOS implementations to federated packages." causes compileSdkVersion issues in APK builds

Steps to Reproduce
using pub.dev plugins that depend on https://pub.dev/packages/video_player/changelog 2.2.18+ causes android APK to depend on compileSdkVersion 31
version numbering should have reflected that with major update (like 2.3.0) as using dependency markup like
video_player: ^2.2.7

causes 2.2.18 to be taken into use automatically
Logs

Logs
FAILURE: Build failed with an exception.

* What went wrong:

Execution failed for task ':app:checkDebugAarMetadata'. > A failure occurred while executing com.android.build.gradle.internal.tasks.CheckAarMetadataWorkAction > The minCompileSdk (31) specified in a

dependency's AAR metadata (META-INF/com/android/build/gradle/aar-metadata.properties) is greater than this module's compileSdkVersion (android-30). Dependency: androidx.core:core:1.7.0.

AAR metadata file: /Users/.gradle/caches/transforms-2/files-2.1/1978c8540575e4da29114a40e6bf1a1a/core-1.7.0/META-INF/com/android/build/gradle/aar-metadata.properties.

Hi @haamaanaa, I am quite not sure what this issue is proposing,

version numbering should have reflected that with major update (like 2.3.0) as using dependency markup like

Do you mean that the version 2.2.18+ requires to depend on compile Sdk 31 so the version should have been 2.3.0 ?

Hi @haamaanaa, I am quite not sure what this issue is proposing,

version numbering should have reflected that with major update (like 2.3.0) as using dependency markup like

Do you mean that the version 2.2.18+ requires to depend on compile Sdk 31 so the version should have been 2.3.0 ?

yeah, exactly. Seems to me having been API level breaking change and as such could have been a major(er?) version update.

Thanks for the clarification @haamaanaa, Considering the current version is 2.4.0, I am quite not sure if it makes sense to update the older version, Perhaps updating the changelog to indicate that the app must use minSdk 31  in 2.2.18 release might help. I will leave this issue open for the team to decide.
cc: @stuartmorgan

yeah, wasn't expecting to change happen in history of time :)
Just that maybe next time API breaking changes happen, this would be notified with major version change.
If there is possibilities to add info into to changeLog, that would probably be enough


Rotate by 90

🚀 Feature
Inplace rotation of matrix by 90 degrees.
Have you read the Contributing Guidelines on Pull Requests?
Yes
Motivation
A hard problem and also has been asked in google interview.
Pitch
A hard problem and also has been asked in google interview.

/assign

I am a gssoc21 participant and I am working on this.


Missing "since" argument error

Hi, I juist installed ORTHANC  with default install.
I am using OHIF embedded viewer in a html page.
I am getting the next error.
What can I fix it?
W0801 11:30:54.957844 main.cpp:1109] DICOM server listening with AET ORTHANC on port: 4242
W0801 11:30:54.957844 HttpServer.cpp:1298] HTTP compression is enabled
W0801 11:30:54.957844 main.cpp:1011] Remote LUA script execution is disabled
W0801 11:30:54.957844 HttpServer.cpp:1205] HTTP server listening on port: 8042 (HTTPS encryption is disabled, remote access is not allowed)
W0801 11:30:54.957844 main.cpp:814] Orthanc has started
E0801 11:37:43.686146 OrthancException.h:76] Bad request: Missing "since" argument for GET request against: /studies/

window.OHIFViewer.installViewer(
      {
        routerBasename: "/",
        extensions: [],
        showStudyList: true,
        filterQueryParam: false,
        servers: {
          dicomWeb: [
            {
              name: "ORTHANC",
              wadoUriRoot: "http://localhost:8042/wado",
              qidoRoot: "http://localhost:8042/dicom-web",
              wadoRoot: "http://localhost:8042/dicom-web",
              qidoSupportsIncludeField: false,
              imageRendering: "wadors",
              thumbnailRendering: "wadors",
            },
          ],
        },
      },
      containerId,
      componentRenderedOrUpdatedCallback
    );
  },

I have the same exact issue. Any news?

In the Chrome debugger the error I see is:
GET http://my.orthancsrv.com/Orthanc/studies?limit=25&offset=0&includefield=all&StudyDate=19531101-20220413 400 (Bad Request)
While according to Orthanc docs (https://api.orthanc-server.com/index.html#tag/Studies/paths/~1studies/get) the parameter should have been named "since" and not "offset",

facing the same problem Bad request: Missing "since" argument for GET request against: /studies/
Using the latest versions of Orthanc and OHIF.
Has anyone managed to get around it somehow?

I think you are looking at the Orthanc API. OHIF does not use the Orthanc API, it uses the API provided by the Orthanc dicomweb plugin. The endpoints have similar names, but you are probably missing a prefix to the dicomweb endpoint.
If someone wants to write an OHIF data source to use the Orthanc API that may also be an option, but it's not a priority on our end.

Please review the latest code in the master branch. I am confident that this issue has been resolved. If it persists, kindly reopen the issue with updated details.
Try viewer-dev.ohif.org instead of viewer.ohif.org
Our viewer.ohif.org is deployed from release branch while viewer-dev.ohif.org is our master  branch
Read more about branch explanations here https://docs.ohif.org/development/getting-started#developing

Perfect, I ended up forgetting to come back here and give feedback. @swederik was precise in his comment, it wasn't a bug, it was hitting the Orthanc API instead of dicomweb which has some similar endpoints, hence the confusion. Thanks


WGan-gp test in the Celeba dataset.

I test the wgan-gp in the celeba dataset.
But the quality of the generative images is worse than the original dcgan.
and i just change the below code in the basic of w-gan using dcgan generator and discirmator.

#gradient penalty
differences = self.fake_images - self.images
 alpha = tf.random_uniform(shape=[self.batch_size, 1], minval=0., maxval=1.)
 interpolates = self.images + (alpha*differences)
gradients = tf.gradients(self.critic(interpolates, True), [interpolates])[0]
 ##2 norm
 slopes = tf.sqrt(tf.reduce_sum(tf.square(gradients), reduction_indices=[1]))
 gradient_penalty = tf.reduce_mean((slopes - 1.)**2)

And the reason?

That code looks correct. It's hard to say without seeing the rest of the code, but if you point me to the repo I can try and debug.

This repo , https://github.com/zhangqianhui/wgan-gp-debug
Thank you !

The sample of gerneration
in epoch 4 , iter= 12601
https://github.com/zhangqianhui/wgan-gp-debug/blob/master/sample/train_04_12601.png
in epoch 6, iter= 20401
https://github.com/zhangqianhui/wgan-gp-debug/blob/master/sample/train_06_20401.png

Could you share the learning curve? (I.e. negative of the critic's loss)

the curve:
https://github.com/zhangqianhui/wgan-gp-debug/blob/master/sample/curve.png

That doesn't look good. @igul222 did you ever see something like that?
Could you share the full code?
Best :)
Martin

@martinarjovsky  Whose code ?

Yours!

Don't know whether it is related but, in my experiments of wgan-gp the loss of G becomes negative, which is different from original wgan in which loss of G is generally positive. Is that normal?

https://github.com/zhangqianhui/wgan-gp-debug @martinarjovsky

@igul222  @martinarjovsky  hello , have you found the reason about the face generation for lower quality?

@martinarjovsky  OK

@martinarjovsky  But , his code have not trained in celeba dataset , so Which architecture i need to use ? Is it Ok to use gan_64x64.py and dcgan's architecure?

That should be fine.

I test in this project, and it can generate very realistic face image after training in the celeba data-set.
But,I can't find reason that my code don't work the same as it.

Here are some differences I found between your implementation and ours which might be responsible:

Gan.py#141: You train the critic for 100 iters every 500 steps. We don't do this, and it's probably responsible for the spikes in the loss curve. Try removing it.
Gan.py#61: It looks like self.images and self.fake_images both have shape [self.batch_size, 64, 64, self.channel]. In this case, alpha should have shape [self.batch_size, 1, 1, 1], and also reduction_indices on line 67 should be [1,2,3].
Because you're not using any normalization, you might want to check your weight initializations. We use the initialization in https://arxiv.org/abs/1502.01852 everywhere. Alternately, you might want to just use normalization: in our default LSUN implementation we use batch normalization in the generator and layer normalization in the critic.

Hope this helps!

@igul222  Thanks , I have solved this problem.!

Cool! What was the issue?

@martinarjovsky  igul222: Gan.py#61: It looks like self.images and self.fake_images both have shape [self.batch_size, 64, 64, self.channel]. In this case, alpha should have shape [self.batch_size, 1, 1, 1], and also reduction_indices on line 67 should be [1,2,3].

and my nextbatch() also have som problem.

And I think layer normalization is very important.
Thanks!
@igul222  @martinarjovsky

Hi @zhangqianhui Im new to WGAN-GP
Im wondering that if we define G_Loss as g_loss = -D(Fake_Image) ,  we're expecting a loss to converge and minimize to a number in loss curve and a maximized loss curve while define G_Loss as g_loss = D(Fake_Image) ?
In C_Loss(Critic loss), if we define C_Loss as c_loss = D(Fake_Image)-D(Real_Image) does that mean we're expecting a loss to converge and minimize to a number and c_loss = D(Fake_Image)-D(Real_Image) is what you called "negative of critic loss"?

the critic loss should be negative, because  the critic loss means the negative of the divergence between the real samples distribution and fake samples distribution. You should read the wgan-gp paper for more details. And g_loss= - c_loss = - D(fake_image) + D(real_image), but the gradient of D(real_images) will not affect g network, so, g_loss= -D(fake_image)

@timho102003

@zhangqianhui  Thanks for your prompt attention. I'm trying on combining the gradient penalty with conditional gan to see if the wasserstein loss with gradient penalty actually gives a better training on both D and G and further enhance the classification on face recognition. As your explanation above, the negative critic loss will start from large negative number and finally reach 0 thru training. However mine seems a little bit different. The only benefit i got for now is that the validation on classification is better than without wasserstein loss with gradient penalty. Do u have any experience on this?
Negative Critics Curve : - D(fake_image) + D(real_image)
https://imgur.com/f8kXacz
D Wasserstein Loss:
https://imgur.com/eOnuvXk

Doing Classification after using training wgan?

Actually I've done the multi-task on Discriminator to not only determine the real/fake problem but also classify Identities and other informations from face which has already come up with a good performance on the protocol such as MPIE. So far I'm trying on merging the WGAN to the implementation.
In my network, there's a average pooling(the output is Bx320x1x1)before a fc(in my previous version, fully connected layer do the multi-task work such as real/fake, Identity...). I take away the real/fake work from fully connected layer and use "view" function to reshape the dimension of average pooling(Bx320x1x1->BX320) and serve as the output of discriminator while calculating Wasserstein Loss.
From my previous experience, I feel like the model is training on the right direction. However  the negative critic loss didn't start from large negative number and finally reach 0 thru training.
Generated Image (20epoch): Number of critic=5 while training, LR=0.0001
https://imgur.com/a/QVizP
Negative Critics Curve : - D(fake_image) + D(real_image):
https://imgur.com/a/jLn3q
D Wasserstein Loss: D(fake_image) - D(real_image) +Gradient Penalty
https://imgur.com/a/G5Wvl
G Wasserstein Loss: - D(fake_image)
https://imgur.com/a/F535n

hi,now i am also trying to train on CelebA (cropped and resized to 64x64) in WGAN-GP mode . I just modify the DATA_DIR in gan_64x64.py. But there was a mistake like this：
IOError: [Errno 2] No such file or directory: '/data-4T-B/yelu/data/dcgan-completion.tensorflow/aligned/img_align_celeba_png/train_64x64/train_64x64/0927649.png'
Could you show me your code? thanks so much~~~


Cluster node runs out of EBS disk space

The worker nodes on the HyperPod cluster currently have a fixed root volume size of 100GB and may run out of disk space when performing large docker/pyxis builds, for example, which use are configured to use the root volume by default.

Support for larger root volume size is in the works.  In the meantime, you can use the FSx for Lustre shared file system - or the local  NVMe volume as a workaround.
You would need to modify the docker working directory to the /fsx mount or NVMe local disk.  Or you can also mount your home directory to /fsx or NVMe, as well.